Skip to content

Commit 0118ee0

Browse files
maunilmclaude
andcommitted
test(security): fix an intermittent failure in the extraction-guard suite [DEVA11Y-484]
A reviewer running the suite alongside another run saw it fail once in seven, and the cause is real: fixture generation is not atomic, and run_tests.sh gated on "does legit.tar.gz exist?" — which make_fixtures.sh creates FIRST. A second run starting behind a generating one saw the gate satisfied and began reading bomb/manyfiles/multifile while they were still being written. Reproduced deterministically: clear the fixtures, start one run, start a second 3 seconds later, and the second fails 5 of 51 assertions — many-files and multi-file across all three variants, which are exactly the fixtures written last. Generation now takes an atomic mkdir lock (macOS ships no flock CLI) and writes a `.complete` marker as its final act; run_tests.sh gates on that marker and hard- fails if it is still absent afterwards. A run that loses the lock waits for the marker rather than proceeding on half-written input. Validated: 4 concurrent cold starts, a staggered cold start, and 6 warm serial runs — 51/51 every time, lock cleaned up, fixtures still gitignored. Mutation validation re-run and unchanged: disabling the decompressed-size rejection, raising the cap to 4 GB, dropping the head -c truncation, and removing both compressed-cap layers are each still caught. Also documents why the corrupt fixture uses /dev/urandom rather than /dev/zero: an all-zero file is a VALID EMPTY tar archive, so zeros make bsdtar exit 0 and the corrupt assertions pass for the wrong reason. That one bit me while building this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3027e92 commit 0118ee0

3 files changed

Lines changed: 54 additions & 1 deletion

File tree

tests/extraction-guard/README.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,16 @@ test. The suite asserts on the *message*, which differs per path.
4949
**The expected file mode is read from `cli.sh`**, not hardcoded: main tightened it
5050
from `0775` to `0755` and a hardcoded expectation had already rotted.
5151

52+
**Fixture generation is locked and marker-gated.** Generation is not atomic, and
53+
the first version gated on "does `legit.tar.gz` exist?" — which `make_fixtures.sh`
54+
creates *first*. A second run starting behind a generating one therefore saw the
55+
gate satisfied and read `bomb`/`manyfiles`/`multifile` while they were still being
56+
written, failing 5 of 51 assertions. This was not theoretical: a reviewer running
57+
the suite alongside another run hit it (1 run in 7). Generation now takes an
58+
atomic `mkdir` lock and writes a `.complete` marker last; `run_tests.sh` gates on
59+
that marker. Validated with 4 concurrent cold starts, a staggered cold start, and
60+
6 warm serial runs — all green.
61+
5262
## Mutation-validated
5363

5464
Baseline green; each guard removal below flips it red:

tests/extraction-guard/make_fixtures.sh

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,39 @@
22
# Generates real .tar.gz fixtures for the DEVA11Y-484 extraction-guard tests.
33
# Everything is bounded: even if a guard regressed and failed to abort, no fixture
44
# decompresses beyond ~400 MB, so a test run can never exhaust the disk.
5+
#
6+
# Generation is LOCKED and gated by a `.complete` marker written last, because it is
7+
# not atomic and a concurrent reader will otherwise consume half-written files.
8+
# Measured: gating on "does legit.tar.gz exist?" let a second run — started 3s behind
9+
# a generating one — fail 5 of 51 assertions, because legit.tar.gz is created FIRST
10+
# and the later fixtures were still being written. That is a real intermittent
11+
# failure, first observed by a reviewer running the suite alongside another run.
512
set -euo pipefail
613

714
DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/fixtures"
815
mkdir -p "$DIR"
16+
17+
MARKER="$DIR/.complete"
18+
LOCK="$DIR/.lock"
19+
20+
# mkdir is atomic on every POSIX filesystem, so it serves as a lock without flock,
21+
# which macOS does not ship as a CLI.
22+
if ! mkdir "$LOCK" 2>/dev/null; then
23+
echo " another run is generating fixtures; waiting..."
24+
for _ in $(seq 1 300); do
25+
[ -f "$MARKER" ] && { echo " fixtures ready (generated by the other run)."; exit 0; }
26+
sleep 1
27+
done
28+
echo "ERROR: timed out waiting for fixtures. Remove $LOCK if it is stale." >&2
29+
exit 1
30+
fi
31+
trap 'rmdir "$LOCK" 2>/dev/null || true' EXIT
32+
33+
if [ -f "$MARKER" ]; then
34+
echo " fixtures already complete."
35+
exit 0
36+
fi
37+
938
cd "$DIR"
1039

1140
log() { printf ' %s\n' "$*"; }
@@ -62,6 +91,9 @@ make_oversized_download() {
6291
}
6392

6493
# --- 6. corrupt: not a valid archive (bsdtar should fail cleanly) ---
94+
# /dev/urandom, not /dev/zero: an all-zero file is a VALID EMPTY tar archive (tar
95+
# terminates on two zero blocks), so zeros would make bsdtar exit 0 and the "corrupt"
96+
# assertions would pass for the wrong reason — measured while building this suite.
6597
make_corrupt() {
6698
head -c 4096 /dev/urandom > corrupt.tar.gz
6799
log "corrupt.tar.gz (random bytes, not a real archive)"
@@ -74,4 +106,7 @@ make_manyfiles
74106
make_multifile
75107
make_oversized_download
76108
make_corrupt
109+
110+
# Marker LAST. A reader gated on this can never observe a partially written fixture.
111+
touch "$MARKER"
77112
echo "Done."

tests/extraction-guard/run_tests.sh

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,20 @@ for tool in bsdtar curl python3 awk; do
2525
}
2626
done
2727

28-
if [ ! -f "$HERE/fixtures/legit.tar.gz" ]; then
28+
# Gate on the completion marker, NOT on any individual fixture. legit.tar.gz is
29+
# written first, so gating on it lets a concurrent run start reading while the later
30+
# fixtures are still being written — measured: 5 of 51 assertions failed that way.
31+
if [ ! -f "$HERE/fixtures/.complete" ]; then
2932
echo
3033
echo "▶ Generating fixtures (~106 MB, gitignored)"
3134
bash "$HERE/make_fixtures.sh" || exit 1
3235
fi
3336

37+
if [ ! -f "$HERE/fixtures/.complete" ]; then
38+
echo "FATAL: fixtures incomplete after generation." >&2
39+
exit 1
40+
fi
41+
3442
echo
3543
echo "▶ Shell launcher tests (bash / zsh / fish)"
3644
bash "$HERE/test_shell_extraction.sh" || rc=1

0 commit comments

Comments
 (0)