diff --git a/.github/workflows/spm-smoke-test.yml b/.github/workflows/spm-smoke-test.yml index 7a94e50..796f4a1 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 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. diff --git a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift index 2da79a7..3c2e867 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) @@ -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 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..6c200ed --- /dev/null +++ b/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"