From 4ab211fd26ec1f737aedd4bb559ec31180e13f2d Mon Sep 17 00:00:00 2001 From: cschantz Date: Fri, 9 Jan 2026 18:06:27 -0500 Subject: [PATCH] Fix false positives in QA checks SUBSHELL-VAR (CHECK 69): - Skip variables only used for writing to files (echo ... >> pattern) - File writes persist even in subshells, so these are safe NULL (CHECK 47): - Skip echo/print_info/print_warning/print_error/printf statements - These are displaying example commands, not executing them ESCAPE (CHECK 66): - Skip filename variables after redirection operators (>, >>, 2>) - Example: grep ... > "$output_file" is writing TO file, not reading FROM it These improvements reduce false positive rate significantly. --- tools/toolkit-qa-check.sh | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tools/toolkit-qa-check.sh b/tools/toolkit-qa-check.sh index a3ed181..dc6151c 100755 --- a/tools/toolkit-qa-check.sh +++ b/tools/toolkit-qa-check.sh @@ -1713,6 +1713,11 @@ while IFS=: read -r file line_num line_content; do # Skip if already suppressed is_suppressed "$file" "$line_num" "null-check" && continue + # Skip if this is just displaying example commands (echo/print_info/print_warning) + if echo "$line_content" | grep -qE '^\s*(echo|print_info|print_warning|print_error|printf)\s+'; then + continue + fi + # Extract variable being used var_name=$(echo "$line_content" | grep -oE '\$\{?[A-Z_a-z][A-Z_a-z0-9]*' | head -1 | tr -d '${}') @@ -2374,6 +2379,12 @@ while IFS=: read -r file line_num line_content; do continue fi + # Skip if filename appears after redirection (output, not input) + # Example: grep ... > "$output_file" (safe - writing to file, not reading from it) + if echo "$line_content" | grep -qE '(>|>>|2>)\s*"\$[a-zA-Z_]*[Ff]ile'; then + continue + fi + var=$(echo "$line_content" | grep -oE '\$[a-zA-Z_]*[Ff]ile[a-zA-Z_]*' | head -1) echo "HIGH|$file|$line_num|[ESCAPE] Filename variable in grep/sed: $var" echo " Risk: If $var='test*.txt', * treated as glob not literal" @@ -2481,6 +2492,13 @@ while IFS=: read -r file line_num line_content; do next_lines=$(sed -n "$((line_num + 1)),$((line_num + 10))p" "$file" 2>/dev/null) if echo "$next_lines" | grep -qE '^[[:space:]]*[a-zA-Z_][a-zA-Z0-9_]*=' && \ echo "$next_lines" | grep -qE 'done'; then + # Skip if variable is only used for file writes (safe pattern) + # Example: username=$(echo "$log" | ...); echo "$log|php_$username" >> "$FILE" + if echo "$next_lines" | grep -qE '[a-zA-Z_][a-zA-Z0-9_]*=.*echo.*>>' || \ + echo "$next_lines" | grep -qE 'echo.*\$[a-zA-Z_][a-zA-Z0-9_]*.*>>'; then + continue # Variable only used for writing to files (changes persist) + fi + echo "HIGH|$file|$line_num|[SUBSHELL-VAR] Variable assignment in pipe (changes lost)" echo " Risk: cmd | while read x; do count=\$((count+1)); done; echo \$count # always 0!" echo " Fix: Use process substitution: while read x; do ...; done < <(cmd)"