Skip to content

Commit 0cc2b6b

Browse files
maunilmclaude
andcommitted
test(security): real-process regression suite for the bsdtar extraction guard [DEVA11Y-484]
Adds local integration tests (no mocks) that exercise the decompression-bomb guards against real curl/bsdtar/head and the real Swift watchdog, plus hardens the guard itself based on what the tests surfaced. Guard hardening (Plugins/BrowserStackAccessibilityLint.swift): - The watchdog now also terminates bsdtar on an entry-count ceiling, closing the "millions of tiny files" bomb that stays small on disk (previously only locateExecutable caught it, after the fact). - Added a post-extraction footprint check so detection is deterministic on fast disks: a bomb that finishes decompressing within a single 200ms poll interval is now caught and cleaned up rather than slipping past the live watchdog. - Refactored the guard into a self-contained, marked block of free functions so it can be mirrored and drift-checked. Tests (scripts/test/, run via run_tests.sh): - Shell: extracts the REAL download_binary from bash/zsh/fish verbatim and runs it against a local server (only the hardcoded URL is redirected, via a curl shim). - Swift: a mirror harness compiles the guard block verbatim and drives real curl/bsdtar; check_drift.sh fails CI if the mirror diverges from the plugin (SwiftPM command plugins can't be imported by a test target). - Scenarios: legit (downloads/extracts/runs), 400MB bomb, 20k-entry bomb, oversized (>100MB) download, corrupt archive, multi-file, missing URL. - Fixtures are bounded (≤400MB, gitignored) and bomb tests use a small cap, so a regressed guard can never exhaust the disk. Full run ~9s, disk usage flat. - CI: .github/workflows/extraction-guard-tests.yml runs the suite on macOS for PRs touching the download/extract path. 53/53 assertions green locally; real production artifact (34MB/64MB) verified to pass through the new extraction path and run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7e139ab commit 0cc2b6b

14 files changed

Lines changed: 843 additions & 44 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Regression tests for the DEVA11Y-484 decompression-bomb extraction guard.
2+
# Runs the real-process integration suite (curl/bsdtar/Swift watchdog) on every PR
3+
# that touches the download/extract path. macOS runner: it ships curl, bsdtar
4+
# (libarchive), python3, and the Swift toolchain.
5+
name: Extraction Guard Tests
6+
7+
on:
8+
pull_request:
9+
branches: ["master", "main"]
10+
paths:
11+
- "Plugins/BrowserStackAccessibilityLint/**"
12+
- "scripts/bash/cli.sh"
13+
- "scripts/zsh/cli.sh"
14+
- "scripts/fish/cli.sh"
15+
- "scripts/test/**"
16+
- ".github/workflows/extraction-guard-tests.yml"
17+
push:
18+
branches: ["master", "main"]
19+
20+
permissions:
21+
contents: read
22+
23+
jobs:
24+
extraction-guard:
25+
name: extraction-guard / integration
26+
runs-on: macos-latest
27+
steps:
28+
- uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
29+
- name: Show toolchain
30+
run: |
31+
swift --version
32+
bsdtar --version
33+
curl --version | head -1
34+
python3 --version
35+
- name: Run DEVA11Y-484 extraction-guard regression suite
36+
run: bash scripts/test/run_tests.sh

Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift

Lines changed: 90 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -275,10 +275,8 @@ private struct BrowserStackCLIDownloader {
275275
throw PluginError("Unable to launch bsdtar: \(error.localizedDescription)")
276276
}
277277

278-
// bsdtar writes decompressed bytes straight to disk, so a cap on the curl→bsdtar
279-
// pipe would only bound the *compressed* size — useless against a decompression
280-
// bomb. Guard the *decompressed* footprint instead (DEVA11Y-484).
281-
let limitState = enforceExtractionSizeLimit(on: bsdtar, extractingInto: directory)
278+
// See the DEVA11Y-484 EXTRACTION GUARD block below for the rationale.
279+
let limitState = startExtractionWatchdog(on: bsdtar, directory: directory, maxBytes: Self.maxDecompressedBytes, maxEntries: Self.maxArchiveEntries)
282280

283281
do {
284282
try curl.run()
@@ -292,9 +290,13 @@ private struct BrowserStackCLIDownloader {
292290
pipe.fileHandleForWriting.closeFile()
293291
bsdtar.waitUntilExit()
294292

293+
// Catch a bomb that completed within a single watchdog poll interval (fast disk).
294+
if !limitState.exceeded, let reason = footprintExceeded(at: directory, maxBytes: Self.maxDecompressedBytes, maxEntries: Self.maxArchiveEntries) {
295+
limitState.markExceeded(reason)
296+
}
295297
if limitState.exceeded {
296298
try? fileManager.removeItem(at: directory)
297-
forwardExit(code: 1, message: "BrowserStack CLI archive exceeds the maximum allowed decompressed size of \(Self.maxDecompressedBytes / (1024 * 1024)) MB. Aborting to prevent disk exhaustion.")
299+
forwardExit(code: 1, message: "BrowserStack CLI archive rejected: \(limitState.reason). Aborting to prevent disk exhaustion.")
298300
}
299301

300302
if curl.terminationStatus != 0 {
@@ -308,39 +310,6 @@ private struct BrowserStackCLIDownloader {
308310
}
309311
}
310312

311-
/// Starts a background watchdog that terminates `bsdtar` if the decompressed footprint in
312-
/// `directory` exceeds `maxDecompressedBytes`. Returns the shared state to inspect afterwards.
313-
private func enforceExtractionSizeLimit(on bsdtar: Process, extractingInto directory: URL) -> ExtractionLimitState {
314-
let state = ExtractionLimitState()
315-
let watchdog = Thread {
316-
while bsdtar.isRunning {
317-
if Self.directorySize(at: directory) > Self.maxDecompressedBytes {
318-
state.markExceeded()
319-
bsdtar.terminate()
320-
break
321-
}
322-
Thread.sleep(forTimeInterval: 0.2)
323-
}
324-
}
325-
watchdog.start()
326-
return state
327-
}
328-
329-
private static func directorySize(at url: URL) -> Int64 {
330-
let fm = FileManager.default
331-
guard let enumerator = fm.enumerator(at: url, includingPropertiesForKeys: [.isRegularFileKey, .fileSizeKey]) else {
332-
return 0
333-
}
334-
var total: Int64 = 0
335-
for case let element as URL in enumerator {
336-
let values = try? element.resourceValues(forKeys: [.isRegularFileKey, .fileSizeKey])
337-
if values?.isRegularFile == true, let size = values?.fileSize {
338-
total += Int64(size)
339-
}
340-
}
341-
return total
342-
}
343-
344313
private func extractLocalArchive(at archiveURL: URL, into directory: URL) throws {
345314
let process = Process()
346315
process.executableURL = URL(fileURLWithPath: "/usr/bin/env")
@@ -351,16 +320,20 @@ private struct BrowserStackCLIDownloader {
351320
let limitState: ExtractionLimitState
352321
do {
353322
try process.run()
354-
// Decompressed-size guard (DEVA11Y-484): same rationale as the remote path.
355-
limitState = enforceExtractionSizeLimit(on: process, extractingInto: directory)
323+
// Decompressed-size/entry guard (DEVA11Y-484): same rationale as the remote path.
324+
limitState = startExtractionWatchdog(on: process, directory: directory, maxBytes: Self.maxDecompressedBytes, maxEntries: Self.maxArchiveEntries)
356325
process.waitUntilExit()
357326
} catch {
358327
throw PluginError("Failed to launch bsdtar: \(error.localizedDescription)")
359328
}
360329

330+
// Catch a bomb that completed within a single watchdog poll interval (fast disk).
331+
if !limitState.exceeded, let reason = footprintExceeded(at: directory, maxBytes: Self.maxDecompressedBytes, maxEntries: Self.maxArchiveEntries) {
332+
limitState.markExceeded(reason)
333+
}
361334
if limitState.exceeded {
362335
try? fileManager.removeItem(at: directory)
363-
forwardExit(code: 1, message: "BrowserStack CLI archive exceeds the maximum allowed decompressed size of \(Self.maxDecompressedBytes / (1024 * 1024)) MB. Aborting to prevent disk exhaustion.")
336+
forwardExit(code: 1, message: "BrowserStack CLI archive rejected: \(limitState.reason). Aborting to prevent disk exhaustion.")
364337
}
365338

366339
if process.terminationReason != .exit || process.terminationStatus != 0 {
@@ -670,14 +643,30 @@ private func isAlpineLinux() -> Bool { false }
670643

671644
// MARK: - Error
672645

646+
// === DEVA11Y-484 EXTRACTION GUARD: shared block ===
647+
// This block is mirrored verbatim in
648+
// scripts/test/swift-harness/Sources/ExtractionHarness/Guard.swift
649+
// so the integration harness exercises the real logic. scripts/test/check_drift.sh
650+
// fails CI if the two copies diverge. Edit both, or neither.
651+
//
652+
// Rationale: bsdtar writes decompressed bytes straight to disk, so a cap on the
653+
// curl→bsdtar pipe would only bound the *compressed* size — useless against a
654+
// decompression bomb. Instead we poll the destination directory while bsdtar runs
655+
// and terminate it if the decompressed footprint crosses a byte OR entry ceiling
656+
// (the entry ceiling stops a "millions of tiny files" bomb that stays small on disk).
657+
673658
/// Thread-safe flag shared between the extraction watchdog and the main flow.
674-
private final class ExtractionLimitState {
659+
final class ExtractionLimitState {
675660
private let lock = NSLock()
676661
private var didExceed = false
662+
private var why = ""
677663

678-
func markExceeded() {
664+
func markExceeded(_ reason: String) {
679665
lock.lock()
680-
didExceed = true
666+
if !didExceed {
667+
didExceed = true
668+
why = reason
669+
}
681670
lock.unlock()
682671
}
683672

@@ -686,7 +675,64 @@ private final class ExtractionLimitState {
686675
defer { lock.unlock() }
687676
return didExceed
688677
}
678+
679+
var reason: String {
680+
lock.lock()
681+
defer { lock.unlock() }
682+
return why
683+
}
684+
}
685+
686+
/// Total bytes and entry count of all regular files under `url`.
687+
func extractionFootprint(at url: URL) -> (bytes: Int64, entries: Int) {
688+
let fm = FileManager.default
689+
guard let enumerator = fm.enumerator(at: url, includingPropertiesForKeys: [.isRegularFileKey, .fileSizeKey]) else {
690+
return (0, 0)
691+
}
692+
var total: Int64 = 0
693+
var count = 0
694+
for case let element as URL in enumerator {
695+
count += 1
696+
let values = try? element.resourceValues(forKeys: [.isRegularFileKey, .fileSizeKey])
697+
if values?.isRegularFile == true, let size = values?.fileSize {
698+
total += Int64(size)
699+
}
700+
}
701+
return (total, count)
702+
}
703+
704+
/// Returns a rejection reason if the footprint under `directory` exceeds either ceiling.
705+
func footprintExceeded(at directory: URL, maxBytes: Int64, maxEntries: Int) -> String? {
706+
let footprint = extractionFootprint(at: directory)
707+
if footprint.bytes > maxBytes {
708+
return "decompressed size exceeds \(maxBytes / (1024 * 1024)) MB"
709+
}
710+
if footprint.entries > maxEntries {
711+
return "archive contains more than \(maxEntries) entries"
712+
}
713+
return nil
714+
}
715+
716+
/// Starts a background watchdog that terminates `process` (bsdtar) if the decompressed
717+
/// footprint in `directory` exceeds the byte or entry ceiling. The watchdog bounds peak
718+
/// disk use during a large/slow bomb; callers MUST also run `footprintExceeded` once the
719+
/// process exits, to catch a fast bomb that finished within a single poll interval.
720+
func startExtractionWatchdog(on process: Process, directory: URL, maxBytes: Int64, maxEntries: Int) -> ExtractionLimitState {
721+
let state = ExtractionLimitState()
722+
let watchdog = Thread {
723+
while process.isRunning {
724+
if let reason = footprintExceeded(at: directory, maxBytes: maxBytes, maxEntries: maxEntries) {
725+
state.markExceeded(reason)
726+
process.terminate()
727+
break
728+
}
729+
Thread.sleep(forTimeInterval: 0.2)
730+
}
731+
}
732+
watchdog.start()
733+
return state
689734
}
735+
// === END DEVA11Y-484 EXTRACTION GUARD ===
690736

691737
private struct PluginError: Error, CustomStringConvertible {
692738
let message: String

scripts/test/.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# Generated by make_fixtures.sh — large/binary, never commit
2+
fixtures/
3+
# SwiftPM build output for the mirror harness
4+
swift-harness/.build/

scripts/test/README.md

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
# DEVA11Y-484 — decompression-bomb guard regression tests
2+
3+
Real, local integration tests for the size/entry guards added to the CLI download path.
4+
**No mocks** — every test runs actual `curl`, `bsdtar`, `head`, and (for the plugin) real
5+
`Process`/watchdog logic against crafted archives served from a local HTTP server.
6+
7+
## Run everything
8+
9+
```bash
10+
scripts/test/run_tests.sh
11+
```
12+
13+
This generates fixtures (first run only), checks guard sync, then runs the shell and
14+
Swift suites. Exit code is non-zero if anything fails.
15+
16+
Requirements: `bash`, `curl`, `bsdtar` (libarchive), `python3`, and the Swift toolchain
17+
(`swift`). All present on the macOS CI image.
18+
19+
## What is covered
20+
21+
| Scenario | Shell (`download_binary`) | Swift plugin (extract paths) |
22+
| --- | --- | --- |
23+
| Legit binary downloads, extracts, **runs**, `0775` |||
24+
| Decompression bomb (400 MB) → abort + cleanup || ✅ (remote + local) |
25+
| Entry-count bomb (20k files) | n/a — `-O` streams, nothing per-entry on disk | ✅ flagged on entry cap |
26+
| Multi-file archive (pre-existing behavior unchanged) |||
27+
| Oversized download (>100 MB) rejected before extraction || ✅ (`curl --max-filesize`) |
28+
| Corrupt archive → clean failure, no false bomb-positive |||
29+
| Missing URL / network failure → abort, no hang |||
30+
31+
All fixtures are **bounded** (nothing decompresses beyond ~400 MB) so a regressed guard
32+
can never exhaust the disk during a test run; bomb tests additionally use a small byte cap.
33+
34+
## Why a "mirror" for the Swift side
35+
36+
SwiftPM **command plugins cannot be imported by a test target** (they run sandboxed and
37+
compile only their own sources), so the plugin's guard logic cannot be unit-tested
38+
directly. Instead:
39+
40+
- The guard lives in a clearly-marked block in
41+
`Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift`
42+
(`=== DEVA11Y-484 EXTRACTION GUARD ===`).
43+
- `swift-harness/Sources/ExtractionHarness/Guard.swift` is a **verbatim mirror** of that
44+
block, compiled into a small executable that drives real `curl`/`bsdtar`.
45+
- `check_drift.sh` diffs the two and **fails if they diverge**, so the mirror can never
46+
silently rot. If you edit the guard, copy the block into both — the drift check enforces it.
47+
48+
## Files
49+
50+
| File | Purpose |
51+
| --- | --- |
52+
| `run_tests.sh` | Orchestrator — run this |
53+
| `make_fixtures.sh` | Generates the bounded test archives into `fixtures/` (gitignored) |
54+
| `check_drift.sh` | Fails if the plugin guard and harness mirror diverge |
55+
| `test_shell_extraction.sh` | Runs the real `download_binary` from all 3 wrappers |
56+
| `test_swift_extraction.sh` | Runs the Swift guard via the mirror harness |
57+
| `lib/assert.sh` | Assertion helpers + local server management |
58+
| `_shim/curl` | Test-only curl shim; redirects the hardcoded URL to the local server |
59+
| `swift-harness/` | Standalone SwiftPM executable mirroring the guard |

scripts/test/_shim/curl

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#!/usr/bin/env bash
2+
# Test-only curl shim. Lets the real, unmodified download_binary() run while
3+
# redirecting ONLY the hardcoded api.browserstack.com download URL to the local
4+
# test server. Every other argument (--max-filesize, -L, -o, -z, ...) is preserved,
5+
# so the security-relevant pipeline is exercised verbatim. Interception happens at
6+
# curl's CLI boundary; the shell function itself is never edited.
7+
set -u
8+
: "${REAL_CURL:?REAL_CURL must point at the real curl}"
9+
: "${TEST_DOWNLOAD_URL:?TEST_DOWNLOAD_URL must be set}"
10+
11+
args=()
12+
for a in "$@"; do
13+
case "$a" in
14+
https://api.browserstack.com/sdk/v1/download_cli*) a="$TEST_DOWNLOAD_URL" ;;
15+
esac
16+
args+=("$a")
17+
done
18+
exec "$REAL_CURL" "${args[@]}"

scripts/test/check_drift.sh

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#!/usr/bin/env bash
2+
# Fails if the DEVA11Y-484 EXTRACTION GUARD block in the SPM plugin has drifted from
3+
# the mirrored copy the Swift harness compiles. SwiftPM command plugins can't be
4+
# imported by a test target, so the harness mirrors the guard verbatim; this check is
5+
# what keeps the mirror honest.
6+
set -euo pipefail
7+
8+
HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
9+
REPO="$(cd "$HERE/../.." && pwd)"
10+
PLUGIN="$REPO/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift"
11+
MIRROR="$HERE/swift-harness/Sources/ExtractionHarness/Guard.swift"
12+
13+
block() {
14+
awk '/=== DEVA11Y-484 EXTRACTION GUARD: shared block ===/,/=== END DEVA11Y-484 EXTRACTION GUARD ===/' "$1"
15+
}
16+
17+
tmp_plugin="$(mktemp)"; tmp_mirror="$(mktemp)"
18+
trap 'rm -f "$tmp_plugin" "$tmp_mirror"' EXIT
19+
block "$PLUGIN" > "$tmp_plugin"
20+
block "$MIRROR" > "$tmp_mirror"
21+
22+
if [ ! -s "$tmp_plugin" ]; then echo "DRIFT CHECK ERROR: guard markers not found in plugin"; exit 2; fi
23+
if [ ! -s "$tmp_mirror" ]; then echo "DRIFT CHECK ERROR: guard markers not found in mirror"; exit 2; fi
24+
25+
if diff -u "$tmp_plugin" "$tmp_mirror"; then
26+
echo "drift check: OK — plugin guard and harness mirror are identical ($(wc -l < "$tmp_plugin" | tr -d ' ') lines)"
27+
else
28+
echo
29+
echo "DRIFT DETECTED: the harness mirror no longer matches the plugin guard block."
30+
echo "Re-sync by copying the block between the markers, then re-run the tests."
31+
exit 1
32+
fi

0 commit comments

Comments
 (0)