From 8f80d267a0709e129f8bcb77864e346e66a666b6 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 17:11:11 +0700 Subject: [PATCH] fleetd #555 rework: catch function definitions after the SOURCED guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comment 17012 on #555 found a hole in test_no_untested_main_flow_conditionals: the guard's function-body detection treats anything inside a function as "fine, out of scope for this scan" — but a function DEFINED after the SOURCED guard line can never be reached by sourcing this script (sourcing stops before the main flow runs), so its body is untestable by construction while still reading to the guard as safely inside a function. mainflow_bare_conditionals now also emits a FUNC record for every function opened after the guard line (reusing the same open-brace detection already used for depth tracking), and test_no_untested_main_flow_conditionals treats any such record as a violation on its own, independent of what the function's body contains or whether the allowlist would otherwise excuse a bare conditional inside it. Proof (redeploy-fleetd.sh restored to 4ffacc5185807d39720a3484d85b922413806eb5347318265bd8897dfd61e8d9 after each): - CONTROL — a bare conditional appended to the main flow is still caught: EXIT=1, "found 1 untested main-flow if/elif/case line(s) ... line 1341: if [ "$MY_CONTROL_BARE" = 1 ]; then :; fi" - CANDIDATE — the same conditional wrapped in a function defined after the boundary, previously invisible (EXIT=0), is now caught: EXIT=1, "line 1341: function defined after the SOURCED guard (line 1038) — it cannot be sourced, so it cannot be tested: newfunc_below_the_boundary() {" Full suite re-run green on both bash 5.3.9 and /bin/bash 3.2.57 (macOS system bash): exit 0, 0 FAIL lines, reached the final PASS line, on both. No change to redeploy-fleetd.sh; the 8 lifted decisions, their mutation proofs, and the allowlist all stand as before. --- scripts/test-redeploy-fleetd.sh | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/scripts/test-redeploy-fleetd.sh b/scripts/test-redeploy-fleetd.sh index 8351214..439e831 100755 --- a/scripts/test-redeploy-fleetd.sh +++ b/scripts/test-redeploy-fleetd.sh @@ -1306,9 +1306,17 @@ MAIN_FLOW_ALLOWED_CONDITIONALS=( 'elif [ "$REDEPLOY_UNEXPLAINED_ERRORS" -eq 0 ]; then' ) -# Emits "\t" for every if/elif/case found outside a function, from guard_line +# Emits "\tCOND\t" for every if/elif/case found outside a function, and +# "\tFUNC\t" for every function DEFINITION found after guard_line — from guard_line # onward. A separate function (rather than inlined into the test) so the deliberate-mutation proof # in the PR description can call it directly against a scratch copy of the script. +# +# fleetd #555 rework (comment 17012): a function defined after the SOURCED guard can never be +# reached by `source`-ing this script (sourcing returns before the main flow, and before any code +# below the guard runs), so anything inside such a function is untestable by construction — the +# depth tracker below would otherwise read it as "inside a function, therefore fine" and wave every +# conditional in it through unexamined. Emitting FUNC records lets the caller flag the function +# itself as the violation, independently of whether its body happens to contain a conditional. mainflow_bare_conditionals() { local src="$1" guard_line="$2" in_func=0 lineno=0 line trimmed while IFS= read -r line || [ -n "$line" ]; do @@ -1316,6 +1324,7 @@ mainflow_bare_conditionals() { [ "$lineno" -le "$guard_line" ] && continue if [[ "$line" =~ ^[A-Za-z_][A-Za-z0-9_]*\(\)[[:space:]]*\{[[:space:]]*$ ]]; then in_func=1 + printf '%d\tFUNC\t%s\n' "$lineno" "$line" continue fi if [ "$in_func" = 1 ] && [[ "$line" =~ ^\}[[:space:]]*$ ]]; then @@ -1325,7 +1334,7 @@ mainflow_bare_conditionals() { if [ "$in_func" = 0 ]; then trimmed="$(printf '%s' "$line" | sed -E 's/^[[:space:]]+//')" if [[ "$trimmed" =~ ^(if|elif|case)[[:space:]] ]]; then - printf '%d\t%s\n' "$lineno" "$trimmed" + printf '%d\tCOND\t%s\n' "$lineno" "$trimmed" fi fi done < "$src" @@ -1336,9 +1345,19 @@ test_no_untested_main_flow_conditionals() { guard_line="$(grep -Fn 'if (return 0 2>/dev/null); then' "$src" | head -1 | cut -d: -f1 || true)" [ -n "$guard_line" ] || { fail "could not find the SOURCED guard in redeploy-fleetd.sh"; return 1; } - local violations=0 report="" found_line found_text allowed candidate - while IFS=$'\t' read -r found_line found_text; do + local violations=0 report="" found_line found_kind found_text allowed candidate + while IFS=$'\t' read -r found_line found_kind found_text; do [ -n "$found_line" ] || continue + if [ "$found_kind" = "FUNC" ]; then + # fleetd #555 rework: a function defined after the SOURCED guard line can never be sourced + # by this suite, so it can never be tested — that is a violation on its own, regardless of + # what its body contains or whether the allowlist would otherwise excuse a bare conditional + # inside it. + violations=$((violations + 1)) + report="$report + line $found_line: function defined after the SOURCED guard (line $guard_line) — it cannot be sourced, so it cannot be tested: $found_text" + continue + fi allowed=0 for candidate in "${MAIN_FLOW_ALLOWED_CONDITIONALS[@]}"; do if [ "$found_text" = "$candidate" ]; then @@ -1354,7 +1373,7 @@ test_no_untested_main_flow_conditionals() { done < <(mainflow_bare_conditionals "$src" "$guard_line") if [ "$violations" -gt 0 ]; then - fail "found $violations untested main-flow if/elif/case line(s), not lifted into a tested predicate function and not in MAIN_FLOW_ALLOWED_CONDITIONALS:$report" + fail "found $violations untested main-flow if/elif/case line(s) or function definition(s) after the SOURCED guard, not lifted into a tested predicate function and not in MAIN_FLOW_ALLOWED_CONDITIONALS:$report" fi }