From c8a0ac5fca297dd8d296cef692b4ae6443171cb6 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Thu, 23 Jul 2026 15:30:01 +0000 Subject: [PATCH] Stop a node whose logs cannot be read from counting as a clean node count_log_pattern summed a per-node `docker logs | grep -c ... || true`, so a node the harness could not read contributed zero. The six assert_zero_count callers are negative health assertions -- no panics, no ERROR lines, no AEAD decrypt failures, no rekey msg2 failures -- and a contributed zero reads as clean, so one unreadable node silently weakened the assertion and all of them voided it. This is the family the harness-fallback issue was raised to high priority for, and it is the one remaining audit residual that can make a green rekey run lie about protocol code. The read now fails the count rather than degrading it, printing a sentinel that names the container. Both callers split the declaration from the assignment, because `local c=$(fn)` takes local's exit status and discards the function's -- which is how the original defect stayed invisible. Validated by breaking what it guards rather than by observing green: against absent containers the old reader returns 0 and assert_zero_count passes vacuously, while the new one returns rc=1 and reports a failure. Checked the other direction too, since a fix that reds a legitimately clean run is no use: a genuine zero across readable nodes still returns 0, and the sum is unchanged at 2+1+0 for a shimmed three-node read. --- testing/static/scripts/rekey-test.sh | 34 ++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/testing/static/scripts/rekey-test.sh b/testing/static/scripts/rekey-test.sh index eab2b2a..bc78113 100755 --- a/testing/static/scripts/rekey-test.sh +++ b/testing/static/scripts/rekey-test.sh @@ -285,15 +285,31 @@ phase_result() { fi } -# Count occurrences of a pattern across all node logs +# Count occurrences of a pattern across all node logs. +# +# A node whose logs cannot be read makes the whole count unusable rather than +# contributing 0. The previous form ended each read `| grep -c "$pat" || true`, +# so a failed `docker logs` yielded 0 for that node and the six assert_zero_count +# callers below read a clean result from a node that was never consulted — one +# unreadable node silently weakened the assertion instead of voiding it. +# +# Returns non-zero and prints a sentinel naming the container. Every caller must +# split the declaration from the assignment (`local c` then `c=$(...)`), because +# `local c=$(...)` takes `local`'s exit status and discards this one. count_log_pattern() { local pattern="$1" local total=0 + local node logs count for node in $NODES; do - local count=$(docker logs "fips-node-${node}${FIPS_CI_NAME_SUFFIX:-}" 2>&1 | grep -c "$pattern" || true) + if ! logs=$(docker logs "fips-node-${node}${FIPS_CI_NAME_SUFFIX:-}" 2>&1); then + echo "unreadable:fips-node-${node}${FIPS_CI_NAME_SUFFIX:-}" + return 1 + fi + count=$(grep -c "$pattern" <<<"$logs" || true) total=$((total + count)) done echo "$total" + return 0 } wait_for_log_pattern_count() { @@ -325,7 +341,12 @@ assert_min_count() { local pattern="$1" local min_count="$2" local description="$3" - local count=$(count_log_pattern "$pattern") + local count + count=$(count_log_pattern "$pattern") || { + echo " ✗ $description: node logs unreadable ($count), count not established" + FAILED=$((FAILED + 1)) + return + } if [ "$count" -ge "$min_count" ]; then echo " ✓ $description: $count (>= $min_count)" PASSED=$((PASSED + 1)) @@ -339,7 +360,12 @@ assert_min_count() { assert_zero_count() { local pattern="$1" local description="$2" - local count=$(count_log_pattern "$pattern") + local count + count=$(count_log_pattern "$pattern") || { + echo " ✗ $description: node logs unreadable ($count), zero not established" + FAILED=$((FAILED + 1)) + return + } if [ "$count" -eq 0 ]; then echo " ✓ $description: 0" PASSED=$((PASSED + 1))