From 54eba31646fc4cd42e7cd6b71339aa9bc224cc7b Mon Sep 17 00:00:00 2001 From: Nishant Maurya Date: Thu, 22 Oct 2026 15:28:34 +0530 Subject: [PATCH 1/5] fix(security): allowlist browserstack.com for the download URL override [DEVA11Y-479][DEVA11Y-488] parseOverride() now rejects any --download-url / BROWSERSTACK_A11Y_CLI_DOWNLOAD_URL whose host is not browserstack.com or a subdomain, using the same policy PR #46 applies to redirect hops. The file:// branches in resolveOverrideArtifact() and download() are removed, and download() refuses any URL outside the policy. Adds tests/download-url-policy, which lifts parseOverride verbatim and checks off-domain, userinfo, look-alike, IP, non-https, file:// and bare-path inputs. Wired into spm-smoke-test.yml. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/spm-smoke-test.yml | 13 +++ .../BrowserStackAccessibilityLint.swift | 30 +++---- tests/README.md | 3 +- tests/download-url-policy/run_tests.sh | 86 +++++++++++++++++++ 4 files changed, 111 insertions(+), 21 deletions(-) create mode 100755 tests/download-url-policy/run_tests.sh diff --git a/.github/workflows/spm-smoke-test.yml b/.github/workflows/spm-smoke-test.yml index 8875f20..34c7334 100644 --- a/.github/workflows/spm-smoke-test.yml +++ b/.github/workflows/spm-smoke-test.yml @@ -138,6 +138,19 @@ jobs: - name: Exercise verify_binary_signature (genuine / tampered / ad-hoc / unsigned) run: tests/launcher-signature/run_tests.sh matrix + # Exercises the plugin's download-URL policy (DEVA11Y-479/488), lifted verbatim from + # the plugin source: the override is accepted only for https on browserstack.com or a + # subdomain. No secrets, no network. + download-url-policy: + name: Download URL override policy + runs-on: macos-14 + timeout-minutes: 10 + steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - name: Exercise parseOverride (allowed / off-domain / non-https / file / bare path) + run: tests/download-url-policy/run_tests.sh + # The CDN publishes no checksum, so the launchers and the SwiftPM plugin trust the # binary's code signature, pinned to BrowserStack's Team ID. If a release is ever # signed differently (new team, lapsed certificate, unsigned build), every user's diff --git a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift index 2da79a7..efc0f83 100644 --- a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift +++ b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift @@ -106,16 +106,18 @@ private func parseOverride(urlString: String?) throws -> URL? { guard scheme.lowercased() == "https" else { throw PluginError("Unsupported URL scheme '\(scheme)' in download URL. Only HTTPS is allowed.") } + guard isAllowedRedirectTarget(url) else { + throw PluginError("Unsupported host '\(url.host ?? "")' in download URL. Only browserstack.com hosts are allowed.") + } return url } -// MARK: - Redirect policy (DEVA11Y-473) +// MARK: - Download host policy (DEVA11Y-473, DEVA11Y-479) -/// 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. +/// The CLI archive may only come from HTTPS on browserstack.com or one of its subdomains. +/// This governs both the override URL (`parseOverride`) and every redirect hop. Without it, +/// an override or a hop to an http:// URL (or an unrelated host) would be followed and the +/// CLI archive downloaded from it. private func isAllowedRedirectTarget(_ url: URL) -> Bool { guard url.scheme?.lowercased() == "https", let host = url.host?.lowercased() else { return false @@ -613,12 +615,7 @@ private struct BrowserStackCLIDownloader { #endif private func resolveOverrideArtifact(from url: URL) async throws -> ArtifactInfo { - let resolvedURL: URL - if url.isFileURL { - resolvedURL = url - } else { - resolvedURL = try await followRedirects(for: url) - } + let resolvedURL = try await followRedirects(for: url) let version = extractVersion(from: resolvedURL) ?? "override" return ArtifactInfo(version: version, resolvedURL: resolvedURL, executableName: executableFileName()) } @@ -716,14 +713,7 @@ private struct BrowserStackCLIDownloader { } private func download(from url: URL, to destination: URL) async throws { - if url.isFileURL { - if fileManager.fileExists(atPath: destination.path) { - try fileManager.removeItem(at: destination) - } - try fileManager.copyItem(at: url, to: destination) - return - } - + try requireAllowedRedirectTarget(url) let (tempURL, response) = try await downloadSession.download(from: url) do { try validateRedirects(response: response, requestedURL: url) diff --git a/tests/README.md b/tests/README.md index d4732f7..411046c 100644 --- a/tests/README.md +++ b/tests/README.md @@ -7,7 +7,7 @@ plugin from this repository into real consumer projects and run accessibility sc against sample sources with intentional issues. Each uses a **path dependency** on the repo root (`../..`), so it always exercises the local plugin sources. -**Regression suites** (`extraction-guard/`) test one hardened code path directly — +**Regression suites** (`extraction-guard/`, `download-url-policy/`) test one hardened code path directly — no consumer project, no credentials, no network. | Folder | Kind | What it exercises | @@ -15,6 +15,7 @@ no consumer project, no credentials, no network. | [`spm/`](./spm) | Integration | SwiftPM consumer: package dependency on `AccessibilityDevTools`; the command plugin is invoked with `swift package plugin … scan`. | | [`xcode-app/`](./xcode-app) | Integration | Xcode iOS app (XcodeGen): a pre-compile build phase runs the scan on every build — the official Xcode integration. | | [`extraction-guard/`](./extraction-guard) | Regression | The DEVA11Y-484 decompression-bomb guard in the shell launchers (`scripts/{bash,zsh,fish}/cli.sh`). Run with `bash tests/extraction-guard/run_tests.sh`. | +| [`download-url-policy/`](./download-url-policy) | Regression | The DEVA11Y-479/488 download-URL override policy in the SwiftPM plugin (`parseOverride`). Run with `tests/download-url-policy/run_tests.sh`. | ## Why two integration harnesses diff --git a/tests/download-url-policy/run_tests.sh b/tests/download-url-policy/run_tests.sh new file mode 100755 index 0000000..c6ce60d --- /dev/null +++ b/tests/download-url-policy/run_tests.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# Tests for the SwiftPM plugin's download-URL policy (DEVA11Y-479/488): the +# --download-url / BROWSERSTACK_A11Y_CLI_DOWNLOAD_URL override is accepted only for +# https on browserstack.com or a subdomain. Lifts parseOverride, isAllowedRedirectTarget +# and PluginError verbatim from the plugin source, so the cases run against the shipped +# code rather than a copy. No network, no credentials. +set -uo pipefail + +ROOT=$(cd "$(dirname "$0")/../.." && pwd) +PLUGIN="$ROOT/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift" +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT + +# Lift a top-level declaration (its first line to the first bare }). +extract() { + awk -v start="$1" 'index($0,start)==1{p=1} p{print} p&&/^}$/{exit}' "$PLUGIN" +} + +{ + echo 'import Foundation' + for decl in 'private func parseOverride(' 'private func isAllowedRedirectTarget(' 'private struct PluginError'; do + body=$(extract "$decl") + if [[ -z "$body" ]]; then + echo "could not find '$decl' in $PLUGIN" >&2 + exit 1 + fi + printf '%s\n\n' "$body" + done + cat <<'SWIFT' +// accept: parseOverride returns a URL; reject: it throws. nil input is a no-op. +let cases: [(String?, Bool)] = [ + (nil, true), + ("", true), + ("https://api.browserstack.com/sdk/v1/download_cli?os=macos&os_arch=arm64", true), + ("https://sdk-assets.browserstack.com/cli/binary-1.2.3.zip", true), + ("https://browserstack.com/cli.zip", true), + ("HTTPS://API.BROWSERSTACK.COM/cli.zip", true), + ("https://api.browserstack.com:443/cli.zip", true), + ("https://evil.example/cli.zip", false), + ("https://github.com/browserstack/cli/releases/download/v1/cli.zip", false), + ("https://evilbrowserstack.com/cli.zip", false), + ("https://browserstack.com.evil.example/cli.zip", false), + ("https://api.browserstack.com@evil.example/cli.zip", false), + ("https://api.browserstack.com:x@evil.example/cli.zip", false), + ("https://evil.example/api.browserstack.com/cli.zip", false), + ("https://evil.example/?h=.browserstack.com", false), + ("https://127.0.0.1/cli.zip", false), + ("https://169.254.169.254/latest/meta-data/", false), + ("http://api.browserstack.com/cli.zip", false), + ("file:///tmp/cli.zip", false), + ("file://api.browserstack.com/tmp/cli.zip", false), + ("/tmp/cli.zip", false), + ("ftp://api.browserstack.com/cli.zip", false), + ("https:///cli.zip", false), +] + +var failures = 0 +for (input, wantAccept) in cases { + let accepted: Bool + var policyHolds = true + do { + let url = try parseOverride(urlString: input) + accepted = true + if let url, !isAllowedRedirectTarget(url) { + policyHolds = false + } + } catch { + accepted = false + } + let label = input.map { "'\($0)'" } ?? "nil" + if !policyHolds { + print("FAIL accepted URL fails the host policy: \(label)") + failures += 1 + } else if accepted == wantAccept { + print("ok \(wantAccept ? "accept" : "reject") \(label)") + } else { + print("FAIL want \(wantAccept ? "accept" : "reject"), got \(accepted ? "accept" : "reject") \(label)") + failures += 1 + } +} +print("\(cases.count - failures)/\(cases.count) passed") +exit(failures == 0 ? 0 : 1) +SWIFT +} > "$WORK/policy.swift" || exit 1 + +swiftc -o "$WORK/policy" "$WORK/policy.swift" && "$WORK/policy" From 0126efa81960d7e2cf99175242954cab01774a2d Mon Sep 17 00:00:00 2001 From: Nishant Maurya Date: Thu, 22 Oct 2026 15:45:38 +0530 Subject: [PATCH 2/5] fix(security): reject '.' as a CLI cache version [DEVA11Y-479] extractVersion() accepted a redirect filename ending in '-..zip' as version '.', which resolves to the cache root itself. Reject it, and cover the version parser in tests/download-url-policy. Co-Authored-By: Claude Opus 5.5 --- .../BrowserStackAccessibilityLint.swift | 2 +- tests/download-url-policy/run_tests.sh | 26 +++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift index efc0f83..3c2e867 100644 --- a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift +++ b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift @@ -897,7 +897,7 @@ private func extractVersion(from url: URL) -> String? { // Reject path traversal and non-semver characters let allowed = CharacterSet.alphanumerics.union(CharacterSet(charactersIn: ".-+")) guard version.unicodeScalars.allSatisfy({ allowed.contains($0) }) else { return nil } - guard !version.contains("..") else { return nil } + guard !version.contains(".."), version != "." else { return nil } return version } return nil diff --git a/tests/download-url-policy/run_tests.sh b/tests/download-url-policy/run_tests.sh index c6ce60d..2415941 100755 --- a/tests/download-url-policy/run_tests.sh +++ b/tests/download-url-policy/run_tests.sh @@ -18,7 +18,7 @@ extract() { { echo 'import Foundation' - for decl in 'private func parseOverride(' 'private func isAllowedRedirectTarget(' 'private struct PluginError'; do + for decl in 'private func parseOverride(' 'private func isAllowedRedirectTarget(' 'private func extractVersion(' 'private struct PluginError'; do body=$(extract "$decl") if [[ -z "$body" ]]; then echo "could not find '$decl' in $PLUGIN" >&2 @@ -78,7 +78,29 @@ for (input, wantAccept) in cases { failures += 1 } } -print("\(cases.count - failures)/\(cases.count) passed") +// extractVersion: the version becomes a cache path component, so it must never be a +// traversal segment or the cache root itself. +let versionCases: [(String, String?)] = [ + ("binary-macos-arm64-1.63.0.zip", "1.63.0"), + ("binary-macos-arm64-..zip", nil), + ("binary-macos-arm64-../../../.ssh.zip", nil), + ("binary-macos-arm64-..%2F..%2F.ssh.zip", nil), + ("binary-macos-arm64-1.0..2.zip", nil), + ("binary-macos-arm64-a%2Fb.zip", nil), + ("binary-macos-arm64-.zip", nil), +] +for (name, want) in versionCases { + let got = extractVersion(from: URL(string: "https://sdk-assets.browserstack.com/" + name)!) + if got == want { + print("ok version \(want.map { "'\($0)'" } ?? "rejected") '\(name)'") + } else { + print("FAIL version want \(want ?? "nil"), got \(got ?? "nil") '\(name)'") + failures += 1 + } +} + +let total = cases.count + versionCases.count +print("\(total - failures)/\(total) passed") exit(failures == 0 ? 0 : 1) SWIFT } > "$WORK/policy.swift" || exit 1 From a7125ec4b6bb438e1b36f1d6f9cf66c003cf57c1 Mon Sep 17 00:00:00 2001 From: Nishant Maurya Date: Thu, 22 Oct 2026 16:06:06 +0530 Subject: [PATCH 3/5] ci: drop the download-url-policy job from spm-smoke-test.yml [DEVA11Y-479] Keeps the PR to the scope of the Security retest. The suite stays in tests/download-url-policy and runs locally. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/spm-smoke-test.yml | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/.github/workflows/spm-smoke-test.yml b/.github/workflows/spm-smoke-test.yml index 34c7334..8875f20 100644 --- a/.github/workflows/spm-smoke-test.yml +++ b/.github/workflows/spm-smoke-test.yml @@ -138,19 +138,6 @@ jobs: - name: Exercise verify_binary_signature (genuine / tampered / ad-hoc / unsigned) run: tests/launcher-signature/run_tests.sh matrix - # Exercises the plugin's download-URL policy (DEVA11Y-479/488), lifted verbatim from - # the plugin source: the override is accepted only for https on browserstack.com or a - # subdomain. No secrets, no network. - download-url-policy: - name: Download URL override policy - runs-on: macos-14 - timeout-minutes: 10 - steps: - - name: Checkout - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - - name: Exercise parseOverride (allowed / off-domain / non-https / file / bare path) - run: tests/download-url-policy/run_tests.sh - # The CDN publishes no checksum, so the launchers and the SwiftPM plugin trust the # binary's code signature, pinned to BrowserStack's Team ID. If a release is ever # signed differently (new team, lapsed certificate, unsigned build), every user's From 4f79d41427f6b420e3589a39ba06a281edda0ea3 Mon Sep 17 00:00:00 2001 From: Nishant Maurya Date: Thu, 22 Oct 2026 16:08:19 +0530 Subject: [PATCH 4/5] ci: run tests/download-url-policy in spm-smoke-test.yml [DEVA11Y-479] Restores the download-url-policy job so the override allowlist is checked on every PR. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/spm-smoke-test.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/spm-smoke-test.yml b/.github/workflows/spm-smoke-test.yml index 8875f20..34c7334 100644 --- a/.github/workflows/spm-smoke-test.yml +++ b/.github/workflows/spm-smoke-test.yml @@ -138,6 +138,19 @@ jobs: - name: Exercise verify_binary_signature (genuine / tampered / ad-hoc / unsigned) run: tests/launcher-signature/run_tests.sh matrix + # Exercises the plugin's download-URL policy (DEVA11Y-479/488), lifted verbatim from + # the plugin source: the override is accepted only for https on browserstack.com or a + # subdomain. No secrets, no network. + download-url-policy: + name: Download URL override policy + runs-on: macos-14 + timeout-minutes: 10 + steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - name: Exercise parseOverride (allowed / off-domain / non-https / file / bare path) + run: tests/download-url-policy/run_tests.sh + # The CDN publishes no checksum, so the launchers and the SwiftPM plugin trust the # binary's code signature, pinned to BrowserStack's Team ID. If a release is ever # signed differently (new team, lapsed certificate, unsigned build), every user's From 0b6d3a23492e53baffd661b161a35793b9d48ab2 Mon Sep 17 00:00:00 2001 From: Nishant Maurya Date: Fri, 23 Oct 2026 10:31:03 +0530 Subject: [PATCH 5/5] test(security): exercise RedirectPolicyDelegate's redirect callback offline [DEVA11Y-479][DEVA11Y-488] The suite checked isAllowedRedirectTarget directly but never the delegate that applies it to every redirect hop, so bypassing the delegate kept every case green. Lift RedirectPolicyDelegate from the plugin and drive its willPerformHTTPRedirection callback with an unstarted task: follow allowed https targets, veto http, off-domain, look-alike and file hops. Co-Authored-By: Claude Opus 5.5 --- tests/download-url-policy/run_tests.sh | 46 +++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/tests/download-url-policy/run_tests.sh b/tests/download-url-policy/run_tests.sh index 2415941..6c200ed 100755 --- a/tests/download-url-policy/run_tests.sh +++ b/tests/download-url-policy/run_tests.sh @@ -1,8 +1,8 @@ #!/usr/bin/env bash # Tests for the SwiftPM plugin's download-URL policy (DEVA11Y-479/488): the # --download-url / BROWSERSTACK_A11Y_CLI_DOWNLOAD_URL override is accepted only for -# https on browserstack.com or a subdomain. Lifts parseOverride, isAllowedRedirectTarget -# and PluginError verbatim from the plugin source, so the cases run against the shipped +# https on browserstack.com or a subdomain. Lifts parseOverride, isAllowedRedirectTarget, +# RedirectPolicyDelegate, extractVersion and PluginError verbatim from the plugin source, so the cases run against the shipped # code rather than a copy. No network, no credentials. set -uo pipefail @@ -18,7 +18,7 @@ extract() { { echo 'import Foundation' - for decl in 'private func parseOverride(' 'private func isAllowedRedirectTarget(' 'private func extractVersion(' 'private struct PluginError'; do + for decl in 'private func parseOverride(' 'private func isAllowedRedirectTarget(' 'private final class RedirectPolicyDelegate' 'private func extractVersion(' 'private struct PluginError'; do body=$(extract "$decl") if [[ -z "$body" ]]; then echo "could not find '$decl' in $PLUGIN" >&2 @@ -99,7 +99,45 @@ for (name, want) in versionCases { } } -let total = cases.count + versionCases.count +// RedirectPolicyDelegate applies the policy to every redirect hop: it must forward an +// allowed https target and veto (hand back nil) anything else. The task is never resumed, +// so nothing touches the network. +private let delegate = RedirectPolicyDelegate() +let session = URLSession(configuration: .ephemeral) +let task = session.dataTask(with: URL(string: "https://api.browserstack.com/sdk/v1/download_cli")!) +let redirectCases: [(String, Bool)] = [ + ("https://sdk-assets.browserstack.com/cli/binary-1.2.3.zip", true), + ("https://browserstack.com/cli.zip", true), + ("http://sdk-assets.browserstack.com/cli/binary-1.2.3.zip", false), + ("https://evil.example/cli.zip", false), + ("https://evilbrowserstack.com/cli.zip", false), + ("https://browserstack.com.evil.example/cli.zip", false), + ("file:///tmp/cli.zip", false), +] +for (target, wantFollow) in redirectCases { + let response = HTTPURLResponse( + url: task.originalRequest!.url!, statusCode: 302, httpVersion: "HTTP/1.1", + headerFields: ["Location": target])! + let newRequest = URLRequest(url: URL(string: target)!) + var followed: URLRequest?? = .none + delegate.urlSession(session, task: task, willPerformHTTPRedirection: response, + newRequest: newRequest) { followed = .some($0) } + guard case .some(let next) = followed else { + print("FAIL redirect callback never called for '\(target)'") + failures += 1 + continue + } + let didFollow = next?.url == newRequest.url + if didFollow == wantFollow { + print("ok redirect \(wantFollow ? "follow" : "veto") '\(target)'") + } else { + print("FAIL redirect want \(wantFollow ? "follow" : "veto"), got \(didFollow ? "follow" : "veto") '\(target)'") + failures += 1 + } +} +task.cancel() + +let total = cases.count + versionCases.count + redirectCases.count print("\(total - failures)/\(total) passed") exit(failures == 0 ? 0 : 1) SWIFT