fix(security): allowlist browserstack.com for the CLI download URL override [DEVA11Y-479][DEVA11Y-488]

.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 launchers accept a CLI download only if curl's final URL is https on browserstack.com
   # or a subdomain (DEVA11Y-473): on Linux there is no signature to check, so the accepted risk
   # rests on that. Exercises is_allowed_download_url lifted from each launcher. Offline.

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)
@@ -907,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

tests/README.md

@@ -7,14 +7,15 @@ 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 |
 |---|---|---|
 | [`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
 

tests/download-url-policy/run_tests.sh

@@ -0,0 +1,146 @@
+#!/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,
+# 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
+
+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 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
+      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
+    }
+}
+// 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
+    }
+}
+
+// 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
+} > "$WORK/policy.swift" || exit 1
+
+swiftc -o "$WORK/policy" "$WORK/policy.swift" && "$WORK/policy"