From 6d1b6c6c69b7e269e4734e7a97bf301ec1a44c4a Mon Sep 17 00:00:00 2001 From: Crash0v3rrid3 Date: Wed, 21 Oct 2026 15:56:34 +0530 Subject: [PATCH 1/2] fix(security): fail-closed signature check in the launchers; replace checksum CI jobs [DEVA11Y-474] The bash/zsh/fish launchers verified the CLI against a .sha256 sidecar that the CDN never publishes, and proceeded unverified when it was missing or malformed: the same bypass Security reopened DEVA11Y-473 over. Replace verify_binary_integrity with verify_binary_signature, mirroring the SwiftPM plugin (#46): on macOS, `codesign --verify --strict` against a Developer ID requirement pinned to Team YQ5FZQ855D, run on the extracted binary before chmod/publish. Failure refuses to run and drops the cached zip so the next run re-downloads instead of re-extracting the same payload after a 304. Linux is unsigned and passes through (accepted risk, DEVA11Y-473); any other OS is refused. CI: replace the two checksum jobs, which asserted the fail-open behaviour and waited on sidecars that will never ship, with tests/launcher-signature/run_tests.sh: - matrix: each launcher's function accepts the real CLI and refuses flipped-byte, ad-hoc-signed, unsigned and missing binaries; - published: the latest macOS arm64/x64 releases satisfy the Team-pinned requirement, so a re-signed release turns CI red first. Self-update .sha256 files regenerated for the edited launchers. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/spm-smoke-test.yml | 110 ++++++-------------------- README.md | 2 +- scripts/bash/cli.sh | 98 ++++++++++------------- scripts/bash/cli.sh.sha256 | 2 +- scripts/fish/cli.sh | 98 ++++++++++------------- scripts/fish/cli.sh.sha256 | 2 +- scripts/zsh/cli.sh | 98 ++++++++++------------- scripts/zsh/cli.sh.sha256 | 2 +- tests/launcher-signature/run_tests.sh | 96 ++++++++++++++++++++++ 9 files changed, 246 insertions(+), 262 deletions(-) create mode 100755 tests/launcher-signature/run_tests.sh diff --git a/.github/workflows/spm-smoke-test.yml b/.github/workflows/spm-smoke-test.yml index f74267d..8875f20 100644 --- a/.github/workflows/spm-smoke-test.yml +++ b/.github/workflows/spm-smoke-test.yml @@ -123,95 +123,31 @@ jobs: done exit "$status" - # Functionally exercises verify_binary_integrity (DEVA11Y-473/474) across its full - # fail-open / fail-closed matrix against a local python fixture server. Because the - # control fails OPEN today (no sidecars published yet), a `bash -n` gate alone would - # give ZERO red signal if this function were broken — the regression would only surface - # the day the sidecars go live. This job is the guard against that. No secrets needed. - verify-integrity-fn: - name: CLI binary integrity check (functional matrix) - runs-on: ubuntu-latest - timeout-minutes: 5 + # Functionally exercises the launchers' verify_binary_signature (DEVA11Y-473/474), + # lifted verbatim from each of the bash/zsh/fish copies: the real CLI must pass, and + # flipped-byte, ad-hoc-signed and unsigned copies must be refused (fail closed). A + # `bash -n` gate alone gives no signal if the check were broken. macOS because the + # check shells out to /usr/bin/codesign. No secrets needed. + verify-signature-fn: + name: Launcher signature verification (functional matrix) + runs-on: macos-14 + timeout-minutes: 10 steps: - name: Checkout uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - - name: Exercise verify_binary_integrity (fail-open / fail-closed matrix) - run: | - set -uo pipefail - SRC=scripts/bash/cli.sh - # Lift the two functions verbatim off the branch (name() { ... } to first bare }). - extract() { awk -v fn="$1" 'index($0,fn"() {")==1{p=1} p{print} p&&/^}$/{exit}' "$SRC"; } - eval "$(extract _self_update_sha256)" - eval "$(extract verify_binary_integrity)" - WORK=$(mktemp -d); cd "$WORK" - printf 'REAL-BINARY-PAYLOAD' > good.zip - GOOD=$(_self_update_sha256 good.zip) - mkdir srv - ( cd srv && python3 -m http.server 8799 >/dev/null 2>&1 & echo $! > "$WORK/pid" ) - sleep 1 - BASE="http://127.0.0.1:8799/asset.zip" - pass=0; fail=0 - # NOTE: GitHub's default `shell: bash -e {0}` injects -e, which `set -uo - # pipefail` does not undo. verify_binary_integrity intentionally returns - # non-zero on the fail-closed cases, so capture rc with `&& rc=0 || rc=$?` - # rather than `; rc=$?` — the latter aborts the whole step under -e before - # the `[ "$fail" -eq 0 ]` gate at the end ever runs. - check() { local d="$1" erc="$2" ez="$3" url="$4" out rc z - cp good.zip z.zip - out=$(verify_binary_integrity z.zip "$url" 2>&1) && rc=0 || rc=$? - z=gone; [ -f z.zip ] && z=kept - if [ "$rc" = "$erc" ] && [ "$z" = "$ez" ]; then echo "PASS | $d | rc=$rc zip=$z"; pass=$((pass+1)) - else echo "FAIL | $d | got rc=$rc zip=$z want rc=$erc zip=$ez | $out"; fail=$((fail+1)); fi - } - check "empty resolved URL (skip)" 0 kept "" - printf '%s asset.zip\n' "$GOOD" > srv/asset.zip.sha256 - check "good sidecar" 0 kept "$BASE" - check "good + ?token= query" 0 kept "${BASE}?token=abc" - rm -f srv/asset.zip.sha256 - check "missing sidecar (404)" 0 kept "$BASE" - printf '%s asset.zip\n' "$(echo "$GOOD" | tr 'a-z' 'A-Z')" > srv/asset.zip.sha256 - check "uppercase published hash" 0 kept "$BASE" - printf '%s asset.zip\n' "0000000000000000000000000000000000000000000000000000000000000000" > srv/asset.zip.sha256 - check "wrong checksum" 2 gone "$BASE" - : > srv/asset.zip.sha256 - check "empty sidecar (200)" 2 gone "$BASE" - printf 'AccessDenied' > srv/asset.zip.sha256 - check "malformed 200 (error page)" 0 kept "$BASE" - kill "$(cat "$WORK/pid")" 2>/dev/null || true - echo "----"; echo "PASS=$pass FAIL=$fail" - [ "$fail" -eq 0 ] + - name: Exercise verify_binary_signature (genuine / tampered / ad-hoc / unsigned) + run: tests/launcher-signature/run_tests.sh matrix - # Positive assertion that the SERVER half (DEVA11Y-473/474) has shipped: resolves the - # real (unauthenticated) download redirect to the versioned asset and probes the - # .sha256 sidecar the client verifies against. Advisory (::warning) today because - # the sidecars are not published yet and verification is inert by design; flip the marked - # line to a hard failure once SDK-assets publishes them so this proves verification runs. - sidecar-availability: - name: CLI checksum sidecar published (advisory until server half ships) - runs-on: ubuntu-latest - timeout-minutes: 5 + # 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 + # launcher would refuse to run; this turns that into a red check here first. + published-signature: + name: Published CLI signed by BrowserStack (Team YQ5FZQ855D) + runs-on: macos-14 + timeout-minutes: 10 steps: - - name: Assert .sha256 resolves to a 64-hex digest - run: | - set -uo pipefail - resolved=$(curl -fsSL -o /dev/null -w '%{url_effective}' \ - "https://api.browserstack.com/sdk/v1/download_cli?os=macos&os_arch=arm64" || true) - if [ -z "$resolved" ]; then - echo "::warning::Could not resolve the CLI asset URL; skipping sidecar availability check." - exit 0 - fi - sum_url="${resolved%%\?*}.sha256" - code=$(curl -fsSL -o body.txt -w '%{http_code}' "$sum_url" || echo 000) - # `|| true`: on the expected 403 (server half not shipped) curl writes no - # body.txt, so awk exits non-zero and pipefail propagates it — which under - # the GHA-injected `bash -e` would abort the step and turn this advisory - # ::warning into a red check. Keep it advisory until the sidecars ship. - first=$(awk '{print $1; exit}' body.txt 2>/dev/null | tr 'A-Z' 'a-z' || true) - if [ "$code" = "200" ] && printf '%s' "$first" | grep -Eq '^[0-9a-f]{64}$'; then - echo "Sidecar published and well-formed at ${sum_url}" - else - # TODO(DEVA11Y-473/474 server half): change the next block to `exit 1` once - # SDK-assets publishes .sha256 (public-read). Until then verification - # is inert, so this stays advisory rather than red-blocking every PR. - echo "::warning::No well-formed checksum sidecar at ${sum_url} yet (HTTP ${code}); CLI binary integrity verification is INERT until the server half ships. Flip this check to a hard failure once sidecars are published." - fi + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - name: Verify the latest published macOS arm64 and x64 CLIs + run: tests/launcher-signature/run_tests.sh published diff --git a/README.md b/README.md index d1a632d..e76d82d 100644 --- a/README.md +++ b/README.md @@ -219,7 +219,7 @@ The SwiftPM plugin verifies the BrowserStack CLI binary's code signature before - **Windows:** the Authenticode signature must be valid and issued to BrowserStack, Inc. - **Linux:** the CLI build is not signed, so there is nothing to verify; the plugin relies on HTTPS alone. This residual risk is accepted on DEVA11Y-473 until the Linux build is signed or the API serves a digest. -The plugin downloads only over HTTPS and only follows download redirects to `https://` URLs on `browserstack.com`. The launcher scripts do not yet verify the CLI binary's signature (tracked in DEVA11Y-474). +The plugin downloads only over HTTPS and only follows download redirects to `https://` URLs on `browserstack.com`. The launcher scripts apply the same check on macOS, on every run before the binary is made executable or run: a binary that fails is refused and the cached download is discarded, so the next run fetches a fresh copy. On Linux they rely on HTTPS alone, under the same accepted risk. Zsh ```zsh diff --git a/scripts/bash/cli.sh b/scripts/bash/cli.sh index 5a9cd31..8931cc5 100644 --- a/scripts/bash/cli.sh +++ b/scripts/bash/cli.sh @@ -190,52 +190,29 @@ strip_quarantine() { fi } -# DEVA11Y-473/474: verify the downloaded CLI archive against a server-published -# SHA-256 sidecar before extracting and executing it. api.browserstack.com (the -# control plane) 302-redirects to a versioned, immutable asset on the CDN/S3 (the -# data plane); a checksum published next to that asset lets us detect a tampered -# or corrupted binary before `chmod 0755` + exec. Semantics mirror self-update: -# fail CLOSED on a checksum mismatch, fail OPEN (warn + proceed) when no sidecar -# is published yet, so this stays non-breaking until the SDK-assets team ships the -# sidecars (the server-side half of DEVA11Y-473/474). -verify_binary_integrity() { - local zip_path="$1" resolved_url="$2" sum_url tmp_sum expected actual - if [[ -z "$resolved_url" ]]; then - echo "CLI download: could not resolve asset URL; skipping integrity check (DEVA11Y-473/474)." >&2 - return 0 - fi - # Derive the sidecar from the asset path only. Stripping any query string keeps - # signed/presigned URLs (…zip?token=) from deriving a permanently-404 sidecar - # (…zip?token=.sha256), which would silently disable verification. - sum_url="${resolved_url%%\?*}.sha256" - tmp_sum=$(mktemp "${TMPDIR:-/tmp}/bs-a11y-clisum.XXXXXX") || return 0 - # shellcheck disable=SC2064 - trap "rm -f -- '${tmp_sum}'" RETURN - if ! curl -fsSL --connect-timeout 10 --max-time 30 "$sum_url" -o "$tmp_sum" 2>/dev/null; then - echo "CLI download: no published checksum at ${sum_url}; proceeding WITHOUT integrity verification (DEVA11Y-473/474)." >&2 - return 0 - fi - expected=$(awk '{print $1; exit}' "$tmp_sum" | tr 'A-Z' 'a-z') - actual=$(_self_update_sha256 "$zip_path" | tr 'A-Z' 'a-z') - # A present-but-empty sidecar body is a hard failure: once the server publishes - # checksums, a blank value must not silently downgrade to "no verification". - if [[ -z "$expected" ]]; then - echo "CLI download: empty checksum at ${sum_url}; refusing to use the downloaded binary." >&2 - rm -f -- "$zip_path" - return 2 - fi - # 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. Its first token must not become the - # "expected hash" and hard-fail every client on every run; fail OPEN instead (DEVA11Y-473/474 review). - if ! [[ "$expected" =~ ^[0-9a-f]{64}$ ]]; then - echo "CLI download: malformed checksum at ${sum_url}; proceeding WITHOUT verification (DEVA11Y-473/474)." >&2 - return 0 - fi - if [[ -z "$actual" || "$expected" != "$actual" ]]; then - echo "CLI download: checksum mismatch; refusing to use the downloaded binary." >&2 - echo " expected: ${expected}" >&2 - echo " actual: ${actual:-}" >&2 - rm -f -- "$zip_path" +# DEVA11Y-473/474: verify the extracted CLI binary's code signature before it is made +# executable, published or run, failing CLOSED. The CDN publishes no checksum, so we +# check the signature BrowserStack already ships the binary with: that proves who built +# it, which a compromised CDN/origin cannot forge. Mirrors the SwiftPM plugin. +# - macOS: `codesign --verify --strict` against a Developer ID requirement pinned to +# BrowserStack's Team ID. +# - Linux: the CLI build is unsigned, so there is nothing to verify. Security accepted +# that residual risk on DEVA11Y-473 until the Linux build is signed or the API serves +# a digest (SDK-6482). +verify_binary_signature() { + local binary_path="$1" team_id="YQ5FZQ855D" requirement codesign_out + case "$OS" in + linux) return 0 ;; + macos) ;; + *) + echo "BrowserStack CLI: cannot verify the binary's signature on ${OS}; refusing to run it." >&2 + return 2 + ;; + esac + requirement="=anchor apple generic and certificate 1[field.1.2.840.113635.100.6.2.6] exists and certificate leaf[field.1.2.840.113635.100.6.1.13] exists and certificate leaf[subject.OU] = \"${team_id}\"" + if ! codesign_out=$(/usr/bin/codesign --verify --strict -R "$requirement" "$binary_path" 2>&1); then + echo "BrowserStack CLI signature verification failed; refusing to run the downloaded binary." >&2 + echo " ${codesign_out}" >&2 return 2 fi } @@ -249,7 +226,7 @@ download_binary() { # exit and nothing written to disk. curl documents the flag as a no-op when the length is # unknown (chunked responses), so the explicit size check below backstops that case — # otherwise an attacker-controlled endpoint could exhaust the disk during download, before - # the checksum and the decompression guard ever run (DEVA11Y-484 review). + # the signature check and the decompression guard ever run (DEVA11Y-484 review). local resolved_url curl_status=0 resolved_url=$(curl -fR --max-filesize "$max_compressed" -z "$BINARY_ZIP_PATH" -L "https://api.browserstack.com/sdk/v1/download_cli?os=${OS}&os_arch=${ARCH}" -o "$BINARY_ZIP_PATH" -w '%{url_effective}') || curl_status=$? if [[ $curl_status -ne 0 ]]; then @@ -267,8 +244,8 @@ download_binary() { # Remove unconditionally, including on a transient network error. This deliberately gives # up the -z If-Modified-Since fast path on the next run: a partial write carries a fresh # mtime, so keeping it risks the next -z revalidation getting a 304 and handing a - # truncated archive to verify_binary_integrity. Losing a 304 is cheaper than trusting a - # truncated payload. + # truncated archive to the extractor. Losing a 304 is cheaper than trusting a truncated + # payload. rm -f "$BINARY_ZIP_PATH" return 1 fi @@ -281,13 +258,10 @@ download_binary() { return 1 fi - verify_binary_integrity "$BINARY_ZIP_PATH" "$resolved_url" || return $? - # Extract to a temp path and atomically publish it. `> "$BINARY_PATH"` truncates the - # destination before bsdtar is known to have succeeded, so a corrupt payload — the live - # case today, since no sidecars are published yet and verification fails open — would - # zero out a previously-good cached binary. Stage + mv keeps the cached binary intact - # unless a fresh, extractable payload is in hand (DEVA11Y-473/474 review). + # destination before bsdtar is known to have succeeded, so a corrupt payload would zero + # out a previously-good cached binary. Stage + mv keeps the cached binary intact unless a + # fresh, extractable, correctly signed payload is in hand (DEVA11Y-473/474 review). # # The decompression-bomb guard (DEVA11Y-484) sits on that same staged path: head -c stops # bsdtar via SIGPIPE once the decompressed output reaches the cap, and pipefail surfaces @@ -325,6 +299,16 @@ download_binary() { return 1 fi + # Verify BEFORE chmod/publish. The archive is re-extracted on every run (even on a 304), + # so this also covers a cached zip. On failure drop the zip too, otherwise the next run's + # If-Modified-Since request gets a 304 and keeps re-extracting the same rejected payload. + local verify_status=0 + verify_binary_signature "${BINARY_PATH}.tmp" || verify_status=$? + if [[ $verify_status -ne 0 ]]; then + rm -f "${BINARY_PATH}.tmp" "$BINARY_ZIP_PATH" + return $verify_status + fi + # Clean the staged file up on *any* failure below, not just the size rejection above, # so a failed chmod/mv never leaves a stray ${BINARY_PATH}.tmp in the cache. if ! { chmod 0755 "${BINARY_PATH}.tmp" && mv -f "${BINARY_PATH}.tmp" "$BINARY_PATH"; }; then @@ -355,7 +339,7 @@ if [[ $SUBCOMMAND == "register-pre-commit-hook" ]]; then exit 0 fi -# Abort before executing the CLI if the download or its integrity check failed -# (checksum mismatch returns 2 from download_binary -> DEVA11Y-473/474). +# Abort before executing the CLI if the download or its signature check failed +# (a failed signature check returns 2 from download_binary -> DEVA11Y-473/474). download_binary || exit $? a11y_scan diff --git a/scripts/bash/cli.sh.sha256 b/scripts/bash/cli.sh.sha256 index 59f2384..9b8c3d3 100644 --- a/scripts/bash/cli.sh.sha256 +++ b/scripts/bash/cli.sh.sha256 @@ -1 +1 @@ -1652f1cd582110d30832f48fc931681ee46b3cae0afad156fa64e4031ddf7804 cli.sh +563fafba772a8fa23fc149b514592ea46961c9c202ab2c5a959f347e28839c5f cli.sh diff --git a/scripts/fish/cli.sh b/scripts/fish/cli.sh index 97165cc..824936a 100644 --- a/scripts/fish/cli.sh +++ b/scripts/fish/cli.sh @@ -202,52 +202,29 @@ strip_quarantine() { fi } -# DEVA11Y-473/474: verify the downloaded CLI archive against a server-published -# SHA-256 sidecar before extracting and executing it. api.browserstack.com (the -# control plane) 302-redirects to a versioned, immutable asset on the CDN/S3 (the -# data plane); a checksum published next to that asset lets us detect a tampered -# or corrupted binary before `chmod 0755` + exec. Semantics mirror self-update: -# fail CLOSED on a checksum mismatch, fail OPEN (warn + proceed) when no sidecar -# is published yet, so this stays non-breaking until the SDK-assets team ships the -# sidecars (the server-side half of DEVA11Y-473/474). -verify_binary_integrity() { - local zip_path="$1" resolved_url="$2" sum_url tmp_sum expected actual - if [[ -z "$resolved_url" ]]; then - echo "CLI download: could not resolve asset URL; skipping integrity check (DEVA11Y-473/474)." >&2 - return 0 - fi - # Derive the sidecar from the asset path only. Stripping any query string keeps - # signed/presigned URLs (…zip?token=) from deriving a permanently-404 sidecar - # (…zip?token=.sha256), which would silently disable verification. - sum_url="${resolved_url%%\?*}.sha256" - tmp_sum=$(mktemp "${TMPDIR:-/tmp}/bs-a11y-clisum.XXXXXX") || return 0 - # shellcheck disable=SC2064 - trap "rm -f -- '${tmp_sum}'" RETURN - if ! curl -fsSL --connect-timeout 10 --max-time 30 "$sum_url" -o "$tmp_sum" 2>/dev/null; then - echo "CLI download: no published checksum at ${sum_url}; proceeding WITHOUT integrity verification (DEVA11Y-473/474)." >&2 - return 0 - fi - expected=$(awk '{print $1; exit}' "$tmp_sum" | tr 'A-Z' 'a-z') - actual=$(_self_update_sha256 "$zip_path" | tr 'A-Z' 'a-z') - # A present-but-empty sidecar body is a hard failure: once the server publishes - # checksums, a blank value must not silently downgrade to "no verification". - if [[ -z "$expected" ]]; then - echo "CLI download: empty checksum at ${sum_url}; refusing to use the downloaded binary." >&2 - rm -f -- "$zip_path" - return 2 - fi - # 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. Its first token must not become the - # "expected hash" and hard-fail every client on every run; fail OPEN instead (DEVA11Y-473/474 review). - if ! [[ "$expected" =~ ^[0-9a-f]{64}$ ]]; then - echo "CLI download: malformed checksum at ${sum_url}; proceeding WITHOUT verification (DEVA11Y-473/474)." >&2 - return 0 - fi - if [[ -z "$actual" || "$expected" != "$actual" ]]; then - echo "CLI download: checksum mismatch; refusing to use the downloaded binary." >&2 - echo " expected: ${expected}" >&2 - echo " actual: ${actual:-}" >&2 - rm -f -- "$zip_path" +# DEVA11Y-473/474: verify the extracted CLI binary's code signature before it is made +# executable, published or run, failing CLOSED. The CDN publishes no checksum, so we +# check the signature BrowserStack already ships the binary with: that proves who built +# it, which a compromised CDN/origin cannot forge. Mirrors the SwiftPM plugin. +# - macOS: `codesign --verify --strict` against a Developer ID requirement pinned to +# BrowserStack's Team ID. +# - Linux: the CLI build is unsigned, so there is nothing to verify. Security accepted +# that residual risk on DEVA11Y-473 until the Linux build is signed or the API serves +# a digest (SDK-6482). +verify_binary_signature() { + local binary_path="$1" team_id="YQ5FZQ855D" requirement codesign_out + case "$OS" in + linux) return 0 ;; + macos) ;; + *) + echo "BrowserStack CLI: cannot verify the binary's signature on ${OS}; refusing to run it." >&2 + return 2 + ;; + esac + requirement="=anchor apple generic and certificate 1[field.1.2.840.113635.100.6.2.6] exists and certificate leaf[field.1.2.840.113635.100.6.1.13] exists and certificate leaf[subject.OU] = \"${team_id}\"" + if ! codesign_out=$(/usr/bin/codesign --verify --strict -R "$requirement" "$binary_path" 2>&1); then + echo "BrowserStack CLI signature verification failed; refusing to run the downloaded binary." >&2 + echo " ${codesign_out}" >&2 return 2 fi } @@ -261,7 +238,7 @@ download_binary() { # exit and nothing written to disk. curl documents the flag as a no-op when the length is # unknown (chunked responses), so the explicit size check below backstops that case — # otherwise an attacker-controlled endpoint could exhaust the disk during download, before - # the checksum and the decompression guard ever run (DEVA11Y-484 review). + # the signature check and the decompression guard ever run (DEVA11Y-484 review). local resolved_url curl_status=0 resolved_url=$(curl -fR --max-filesize "$max_compressed" -z "$BINARY_ZIP_PATH" -L "https://api.browserstack.com/sdk/v1/download_cli?os=${OS}&os_arch=${ARCH}" -o "$BINARY_ZIP_PATH" -w '%{url_effective}') || curl_status=$? if [[ $curl_status -ne 0 ]]; then @@ -279,8 +256,8 @@ download_binary() { # Remove unconditionally, including on a transient network error. This deliberately gives # up the -z If-Modified-Since fast path on the next run: a partial write carries a fresh # mtime, so keeping it risks the next -z revalidation getting a 304 and handing a - # truncated archive to verify_binary_integrity. Losing a 304 is cheaper than trusting a - # truncated payload. + # truncated archive to the extractor. Losing a 304 is cheaper than trusting a truncated + # payload. rm -f "$BINARY_ZIP_PATH" return 1 fi @@ -293,13 +270,10 @@ download_binary() { return 1 fi - verify_binary_integrity "$BINARY_ZIP_PATH" "$resolved_url" || return $? - # Extract to a temp path and atomically publish it. `> "$BINARY_PATH"` truncates the - # destination before bsdtar is known to have succeeded, so a corrupt payload — the live - # case today, since no sidecars are published yet and verification fails open — would - # zero out a previously-good cached binary. Stage + mv keeps the cached binary intact - # unless a fresh, extractable payload is in hand (DEVA11Y-473/474 review). + # destination before bsdtar is known to have succeeded, so a corrupt payload would zero + # out a previously-good cached binary. Stage + mv keeps the cached binary intact unless a + # fresh, extractable, correctly signed payload is in hand (DEVA11Y-473/474 review). # # The decompression-bomb guard (DEVA11Y-484) sits on that same staged path: head -c stops # bsdtar via SIGPIPE once the decompressed output reaches the cap, and pipefail surfaces @@ -337,6 +311,16 @@ download_binary() { return 1 fi + # Verify BEFORE chmod/publish. The archive is re-extracted on every run (even on a 304), + # so this also covers a cached zip. On failure drop the zip too, otherwise the next run's + # If-Modified-Since request gets a 304 and keeps re-extracting the same rejected payload. + local verify_status=0 + verify_binary_signature "${BINARY_PATH}.tmp" || verify_status=$? + if [[ $verify_status -ne 0 ]]; then + rm -f "${BINARY_PATH}.tmp" "$BINARY_ZIP_PATH" + return $verify_status + fi + # Clean the staged file up on *any* failure below, not just the size rejection above, # so a failed chmod/mv never leaves a stray ${BINARY_PATH}.tmp in the cache. if ! { chmod 0755 "${BINARY_PATH}.tmp" && mv -f "${BINARY_PATH}.tmp" "$BINARY_PATH"; }; then @@ -367,8 +351,8 @@ if [[ $SUBCOMMAND == "register-pre-commit-hook" ]]; then exit 0 fi -# Abort before executing the CLI if the download or its integrity check failed -# (checksum mismatch returns 2 from download_binary -> DEVA11Y-473/474). +# Abort before executing the CLI if the download or its signature check failed +# (a failed signature check returns 2 from download_binary -> DEVA11Y-473/474). download_binary || exit $? a11y_scan diff --git a/scripts/fish/cli.sh.sha256 b/scripts/fish/cli.sh.sha256 index 6d29f3f..5b9e554 100644 --- a/scripts/fish/cli.sh.sha256 +++ b/scripts/fish/cli.sh.sha256 @@ -1 +1 @@ -eb31e82f018e80c969e3c2b3103e79c8e490135aecfd52e0e78abe7a6c46816d cli.sh +8e8db46547aee7a326ef624342b17dd829db38fd690146a451ecc04a0a87b4a6 cli.sh diff --git a/scripts/zsh/cli.sh b/scripts/zsh/cli.sh index 8b92862..cc56da5 100644 --- a/scripts/zsh/cli.sh +++ b/scripts/zsh/cli.sh @@ -201,52 +201,29 @@ strip_quarantine() { fi } -# DEVA11Y-473/474: verify the downloaded CLI archive against a server-published -# SHA-256 sidecar before extracting and executing it. api.browserstack.com (the -# control plane) 302-redirects to a versioned, immutable asset on the CDN/S3 (the -# data plane); a checksum published next to that asset lets us detect a tampered -# or corrupted binary before `chmod 0755` + exec. Semantics mirror self-update: -# fail CLOSED on a checksum mismatch, fail OPEN (warn + proceed) when no sidecar -# is published yet, so this stays non-breaking until the SDK-assets team ships the -# sidecars (the server-side half of DEVA11Y-473/474). -verify_binary_integrity() { - local zip_path="$1" resolved_url="$2" sum_url tmp_sum expected actual - if [[ -z "$resolved_url" ]]; then - echo "CLI download: could not resolve asset URL; skipping integrity check (DEVA11Y-473/474)." >&2 - return 0 - fi - # Derive the sidecar from the asset path only. Stripping any query string keeps - # signed/presigned URLs (…zip?token=) from deriving a permanently-404 sidecar - # (…zip?token=.sha256), which would silently disable verification. - sum_url="${resolved_url%%\?*}.sha256" - tmp_sum=$(mktemp "${TMPDIR:-/tmp}/bs-a11y-clisum.XXXXXX") || return 0 - # shellcheck disable=SC2064 - trap "rm -f -- '${tmp_sum}'" RETURN - if ! curl -fsSL --connect-timeout 10 --max-time 30 "$sum_url" -o "$tmp_sum" 2>/dev/null; then - echo "CLI download: no published checksum at ${sum_url}; proceeding WITHOUT integrity verification (DEVA11Y-473/474)." >&2 - return 0 - fi - expected=$(awk '{print $1; exit}' "$tmp_sum" | tr 'A-Z' 'a-z') - actual=$(_self_update_sha256 "$zip_path" | tr 'A-Z' 'a-z') - # A present-but-empty sidecar body is a hard failure: once the server publishes - # checksums, a blank value must not silently downgrade to "no verification". - if [[ -z "$expected" ]]; then - echo "CLI download: empty checksum at ${sum_url}; refusing to use the downloaded binary." >&2 - rm -f -- "$zip_path" - return 2 - fi - # 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. Its first token must not become the - # "expected hash" and hard-fail every client on every run; fail OPEN instead (DEVA11Y-473/474 review). - if ! [[ "$expected" =~ ^[0-9a-f]{64}$ ]]; then - echo "CLI download: malformed checksum at ${sum_url}; proceeding WITHOUT verification (DEVA11Y-473/474)." >&2 - return 0 - fi - if [[ -z "$actual" || "$expected" != "$actual" ]]; then - echo "CLI download: checksum mismatch; refusing to use the downloaded binary." >&2 - echo " expected: ${expected}" >&2 - echo " actual: ${actual:-}" >&2 - rm -f -- "$zip_path" +# DEVA11Y-473/474: verify the extracted CLI binary's code signature before it is made +# executable, published or run, failing CLOSED. The CDN publishes no checksum, so we +# check the signature BrowserStack already ships the binary with: that proves who built +# it, which a compromised CDN/origin cannot forge. Mirrors the SwiftPM plugin. +# - macOS: `codesign --verify --strict` against a Developer ID requirement pinned to +# BrowserStack's Team ID. +# - Linux: the CLI build is unsigned, so there is nothing to verify. Security accepted +# that residual risk on DEVA11Y-473 until the Linux build is signed or the API serves +# a digest (SDK-6482). +verify_binary_signature() { + local binary_path="$1" team_id="YQ5FZQ855D" requirement codesign_out + case "$OS" in + linux) return 0 ;; + macos) ;; + *) + echo "BrowserStack CLI: cannot verify the binary's signature on ${OS}; refusing to run it." >&2 + return 2 + ;; + esac + requirement="=anchor apple generic and certificate 1[field.1.2.840.113635.100.6.2.6] exists and certificate leaf[field.1.2.840.113635.100.6.1.13] exists and certificate leaf[subject.OU] = \"${team_id}\"" + if ! codesign_out=$(/usr/bin/codesign --verify --strict -R "$requirement" "$binary_path" 2>&1); then + echo "BrowserStack CLI signature verification failed; refusing to run the downloaded binary." >&2 + echo " ${codesign_out}" >&2 return 2 fi } @@ -260,7 +237,7 @@ download_binary() { # exit and nothing written to disk. curl documents the flag as a no-op when the length is # unknown (chunked responses), so the explicit size check below backstops that case — # otherwise an attacker-controlled endpoint could exhaust the disk during download, before - # the checksum and the decompression guard ever run (DEVA11Y-484 review). + # the signature check and the decompression guard ever run (DEVA11Y-484 review). local resolved_url curl_status=0 resolved_url=$(curl -fR --max-filesize "$max_compressed" -z "$BINARY_ZIP_PATH" -L "https://api.browserstack.com/sdk/v1/download_cli?os=${OS}&os_arch=${ARCH}" -o "$BINARY_ZIP_PATH" -w '%{url_effective}') || curl_status=$? if [[ $curl_status -ne 0 ]]; then @@ -278,8 +255,8 @@ download_binary() { # Remove unconditionally, including on a transient network error. This deliberately gives # up the -z If-Modified-Since fast path on the next run: a partial write carries a fresh # mtime, so keeping it risks the next -z revalidation getting a 304 and handing a - # truncated archive to verify_binary_integrity. Losing a 304 is cheaper than trusting a - # truncated payload. + # truncated archive to the extractor. Losing a 304 is cheaper than trusting a truncated + # payload. rm -f "$BINARY_ZIP_PATH" return 1 fi @@ -292,13 +269,10 @@ download_binary() { return 1 fi - verify_binary_integrity "$BINARY_ZIP_PATH" "$resolved_url" || return $? - # Extract to a temp path and atomically publish it. `> "$BINARY_PATH"` truncates the - # destination before bsdtar is known to have succeeded, so a corrupt payload — the live - # case today, since no sidecars are published yet and verification fails open — would - # zero out a previously-good cached binary. Stage + mv keeps the cached binary intact - # unless a fresh, extractable payload is in hand (DEVA11Y-473/474 review). + # destination before bsdtar is known to have succeeded, so a corrupt payload would zero + # out a previously-good cached binary. Stage + mv keeps the cached binary intact unless a + # fresh, extractable, correctly signed payload is in hand (DEVA11Y-473/474 review). # # The decompression-bomb guard (DEVA11Y-484) sits on that same staged path: head -c stops # bsdtar via SIGPIPE once the decompressed output reaches the cap, and pipefail surfaces @@ -336,6 +310,16 @@ download_binary() { return 1 fi + # Verify BEFORE chmod/publish. The archive is re-extracted on every run (even on a 304), + # so this also covers a cached zip. On failure drop the zip too, otherwise the next run's + # If-Modified-Since request gets a 304 and keeps re-extracting the same rejected payload. + local verify_status=0 + verify_binary_signature "${BINARY_PATH}.tmp" || verify_status=$? + if [[ $verify_status -ne 0 ]]; then + rm -f "${BINARY_PATH}.tmp" "$BINARY_ZIP_PATH" + return $verify_status + fi + # Clean the staged file up on *any* failure below, not just the size rejection above, # so a failed chmod/mv never leaves a stray ${BINARY_PATH}.tmp in the cache. if ! { chmod 0755 "${BINARY_PATH}.tmp" && mv -f "${BINARY_PATH}.tmp" "$BINARY_PATH"; }; then @@ -366,8 +350,8 @@ if [[ $SUBCOMMAND == "register-pre-commit-hook" ]]; then exit 0 fi -# Abort before executing the CLI if the download or its integrity check failed -# (checksum mismatch returns 2 from download_binary -> DEVA11Y-473/474). +# Abort before executing the CLI if the download or its signature check failed +# (a failed signature check returns 2 from download_binary -> DEVA11Y-473/474). download_binary || exit $? a11y_scan diff --git a/scripts/zsh/cli.sh.sha256 b/scripts/zsh/cli.sh.sha256 index 0b00980..63ef9e6 100644 --- a/scripts/zsh/cli.sh.sha256 +++ b/scripts/zsh/cli.sh.sha256 @@ -1 +1 @@ -42c2f590cab5a24594e9f0db5d7ae25dffd95795838184a78d4c302d31945d4a cli.sh +3852bf2d8f2485f38c4d8f3eee30541c2130681599f380890e2ca9848bcf6776 cli.sh diff --git a/tests/launcher-signature/run_tests.sh b/tests/launcher-signature/run_tests.sh new file mode 100755 index 0000000..ac33b46 --- /dev/null +++ b/tests/launcher-signature/run_tests.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# Tests for the launchers' verify_binary_signature (DEVA11Y-473/474). macOS only: the +# check shells out to /usr/bin/codesign. Exercises the function lifted verbatim from each +# launcher copy, so a drift between bash/zsh/fish shows up here. +# +# run_tests.sh matrix genuine CLI passes; flipped-byte, ad-hoc-signed and unsigned +# copies are refused; Linux passes through; unknown OS refused. +# run_tests.sh published the latest published macOS arm64 and x64 CLIs satisfy the +# Team-pinned requirement, so a re-signed release (new team or +# certificate) turns CI red instead of breaking every launcher. +set -uo pipefail + +MODE="${1:-matrix}" +ROOT=$(cd "$(dirname "$0")/../.." && pwd) +LAUNCHERS=(bash zsh fish) +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT +pass=0 +fail=0 + +# Lift verify_binary_signature off a launcher (name() { ... } to the first bare }). +extract() { + awk 'index($0,"verify_binary_signature() {")==1{p=1} p{print} p&&/^}$/{exit}' "$ROOT/scripts/$1/cli.sh" +} + +# Download and extract the latest CLI for an arch; prints the binary's path. +fetch_cli() { + local arch="$1" dir="$WORK/$1" + mkdir -p "$dir" + curl -fsSL --retry 3 --retry-delay 5 \ + "https://api.browserstack.com/sdk/v1/download_cli?os=macos&os_arch=${arch}" -o "$dir/cli.zip" || return 1 + bsdtar -xf "$dir/cli.zip" -C "$dir" || return 1 + find "$dir" -maxdepth 1 -type f -name 'binary-*' | head -n 1 +} + +# check +check() { + local desc="$1" launcher="$2" os="$3" binary="$4" want="$5" fn out rc + fn=$(extract "$launcher") + if [[ -z "$fn" ]]; then + echo "FAIL | $launcher | verify_binary_signature not found in scripts/$launcher/cli.sh" + fail=$((fail + 1)) + return + fi + out=$(OS="$os" bash -c "$fn"$'\n''verify_binary_signature "$1"' _ "$binary" 2>&1) && rc=0 || rc=$? + if [[ "$rc" == "$want" ]]; then + echo "PASS | $launcher | $desc | rc=$rc" + pass=$((pass + 1)) + else + echo "FAIL | $launcher | $desc | got rc=$rc want rc=$want | $out" + fail=$((fail + 1)) + fi +} + +run_matrix() { + local arch genuine size + case "$(uname -m)" in + arm64) arch=arm64 ;; + *) arch=x64 ;; + esac + genuine=$(fetch_cli "$arch") || { echo "FAIL | could not download the CLI"; fail=$((fail + 1)); return; } + + cp "$genuine" "$WORK/flipped" + size=$(wc -c < "$WORK/flipped") + printf '\xff' | dd of="$WORK/flipped" bs=1 seek=$((size / 2)) conv=notrunc 2>/dev/null + cp "$genuine" "$WORK/adhoc" && codesign --force --sign - "$WORK/adhoc" 2>/dev/null + cp "$genuine" "$WORK/unsigned" && codesign --remove-signature "$WORK/unsigned" + + for l in "${LAUNCHERS[@]}"; do + check "genuine CLI ($arch)" "$l" macos "$genuine" 0 + check "flipped byte" "$l" macos "$WORK/flipped" 2 + check "ad-hoc signed" "$l" macos "$WORK/adhoc" 2 + check "unsigned" "$l" macos "$WORK/unsigned" 2 + check "missing file" "$l" macos "$WORK/does-not-exist" 2 + check "linux (accepted risk)" "$l" linux "$WORK/unsigned" 0 + check "unknown OS refused" "$l" "UNKNOWN:Plan9" "$genuine" 2 + done +} + +run_published() { + local arch binary + for arch in arm64 x64; do + binary=$(fetch_cli "$arch") || { echo "FAIL | could not download the $arch CLI"; fail=$((fail + 1)); continue; } + check "latest published CLI ($arch)" bash macos "$binary" 0 + done +} + +case "$MODE" in + matrix) run_matrix ;; + published) run_published ;; + *) echo "usage: $0 [matrix|published]" >&2; exit 64 ;; +esac + +echo "----" +echo "PASS=$pass FAIL=$fail" +[[ $fail -eq 0 && $pass -gt 0 ]] From 97c44ca5a5b2fa212d69946a57d9094a356ca32c Mon Sep 17 00:00:00 2001 From: Crash0v3rrid3 Date: Wed, 21 Oct 2026 16:01:09 +0530 Subject: [PATCH 2/2] test(extraction-guard): load verify_binary_signature; add signature-gate case [DEVA11Y-474] The suite lifted verify_binary_integrity by name, which no longer exists, and drove download_binary with OS=macos and unsigned fixtures that the new signature check refuses before the guard's outcome is observable. Run the guard cases with OS=linux, where the real verify_binary_signature passes through by design, so they still execute only real code. Add case 9 (macOS): the real codesign check must refuse an unsigned payload with exit 2, remove the staged binary and the zip, and leave an existing cached binary intact. 75/75 locally. Co-Authored-By: Claude Opus 5.5 --- tests/extraction-guard/README.md | 2 +- .../extraction-guard/test_shell_extraction.sh | 35 +++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/tests/extraction-guard/README.md b/tests/extraction-guard/README.md index 7145422..1d3567e 100644 --- a/tests/extraction-guard/README.md +++ b/tests/extraction-guard/README.md @@ -39,7 +39,7 @@ ticket's remediation, but a real gap rather than covered behaviour. ## How it avoids testing the wrong thing **Functions are extracted verbatim.** `load_download_binary` lifts -`_self_update_sha256`, `strip_quarantine`, `verify_binary_integrity` and +`_self_update_sha256`, `strip_quarantine`, `verify_binary_signature` and `download_binary` straight out of `cli.sh` and sources them. Loading only `download_binary` leaves the others undefined, the function dies with exit 127, and *every abort assertion passes for the wrong reason* — so all four are loaded. diff --git a/tests/extraction-guard/test_shell_extraction.sh b/tests/extraction-guard/test_shell_extraction.sh index 650156f..f702750 100755 --- a/tests/extraction-guard/test_shell_extraction.sh +++ b/tests/extraction-guard/test_shell_extraction.sh @@ -32,7 +32,7 @@ start_server "$FIXTURES" || exit 1 # leaves them undefined, and the function then dies with exit 127 on the first call # — which looks exactly like a guard rejection and would make every abort assertion # pass for the wrong reason. Load the whole dependency set. -DEPS=(_self_update_sha256 strip_quarantine verify_binary_integrity download_binary) +DEPS=(_self_update_sha256 strip_quarantine verify_binary_signature download_binary) load_download_binary() { local variant="$1" @@ -78,11 +78,17 @@ load_download_binary() { # (bsdtar takes SIGPIPE when `head -c` closes the pipe), so asserting only # "exit 1" still passes with the size guard deleted — verified by mutation test. # The two paths emit different messages, so the message is the discriminator. +# +# OS defaults to linux: the fixtures are not signed BrowserStack binaries, so on macos +# the real signature check (DEVA11Y-474) would refuse every one of them before the +# extraction guard's outcome could be observed. On linux verify_binary_signature passes +# through by design (accepted risk), so the guard is exercised with no stubs. Case 9 +# runs with OS=macos to prove the signature gate is wired into download_binary. run_case() { local fixture="$1" local cache cache="$(mktemp -d "$WORK/cache.XXXX")" - export OS=macos ARCH=arm64 + export OS="${RUN_CASE_OS:-linux}" ARCH=arm64 export BINARY_ZIP_PATH="$cache/browserstack-cli.zip" export BINARY_PATH="$cache/browserstack-cli" TEST_DOWNLOAD_URL="$(url_for "$fixture")" @@ -217,6 +223,31 @@ for variant in bash zsh fish; do assert_status $? 1 "$variant cached-binary: bomb still rejected" assert_eq "$(cat "$GOOD_CACHE/browserstack-cli" 2>/dev/null)" "sentinel" \ "$variant cached-binary: existing good binary left intact" + + # 9. Signature gate (DEVA11Y-474): on macos the real verify_binary_signature runs + # codesign on the extracted payload. legit.zip is not a signed BrowserStack binary, + # so download_binary must refuse it with exit 2 before chmod/publish, remove the + # staged binary AND the zip (so a 304 cannot re-serve it), and leave an existing + # cached binary untouched. Function-level coverage of genuine/tampered/ad-hoc + # binaries lives in tests/launcher-signature. + if [ "$(uname -s)" = "Darwin" ]; then + run_case "legit.tar.gz" + printf 'sentinel' > "$LAST_CACHE/browserstack-cli" + GOOD_CACHE="$LAST_CACHE" + export OS=macos BINARY_ZIP_PATH="$GOOD_CACHE/browserstack-cli.zip" BINARY_PATH="$GOOD_CACHE/browserstack-cli" + TEST_DOWNLOAD_URL="$(url_for legit.zip)" + export TEST_DOWNLOAD_URL + ( PATH="$SHIM_DIR:$PATH"; download_binary ) >/dev/null 2>"$GOOD_CACHE/sig-stderr.txt" + assert_status $? 2 "$variant signature: unsigned payload refused with exit 2" + assert_contains "$(cat "$GOOD_CACHE/sig-stderr.txt")" "signature verification failed" \ + "$variant signature: refused by the SIGNATURE check (not another guard)" + assert_absent "$GOOD_CACHE/browserstack-cli.tmp" "$variant signature: staged binary removed" + assert_absent "$GOOD_CACHE/browserstack-cli.zip" "$variant signature: rejected zip removed" + assert_eq "$(cat "$GOOD_CACHE/browserstack-cli" 2>/dev/null)" "sentinel" \ + "$variant signature: existing good binary left intact" + else + echo " (skipping signature-gate case: needs macOS codesign)" + fi done summary