diff --git a/tools/toolkit-qa-check.sh b/tools/toolkit-qa-check.sh index 429c860..50319c8 100755 --- a/tools/toolkit-qa-check.sh +++ b/tools/toolkit-qa-check.sh @@ -14,7 +14,7 @@ # --summary Summary mode (counts only, no details) # # Features: -# - 88 comprehensive checks (was 80, +8 multi-panel compliance) +# - 98 comprehensive checks (was 88, +6 logic validation, +4 advanced error detection) # - Context-aware detection (<5% false positives) # - Smart categorization with tags # - Suppress annotations support (# qa-suppress) @@ -23,6 +23,8 @@ # - Phase 5: Deep analysis (locale, printf injection, bashisms, etc.) # - Phase 6: Performance & resource checks # - Phase 7: Multi-panel architecture compliance +# - Phase 8: Logic validation (contradictory patterns, type mismatches, uninitialized vars, etc) +# - Phase 9: Advanced error detection (missing error checks, subshell shadowing, array bounds) # # Parse options @@ -3472,6 +3474,159 @@ echo "Found: $count undefined variable references" echo "" } >> "$REPORT" +#============================================================================== +# CHECK 95: Missing Error Checks After Critical Commands (HIGH) +#============================================================================== +show_progress 95 "Missing error checks after critical commands" +{ +echo "## CHECK 95: Missing Error Checks After Critical Commands" +echo "Severity: HIGH" +echo "Pattern: Variable assignment from critical commands without exit validation" +echo "Impact: Silent failures - invalid data used in subsequent operations" +echo "" + +count=0 +# Look for: var=$( mysql/curl/etc command ) without checking if it succeeded +while IFS=: read -r file line_num line_content; do + # Pattern: var=$(mysql ... ) followed by data usage without validation + if echo "$line_content" | grep -qE '=\$\((.*mysql|.*curl|.*wget)' && \ + ! echo "$line_content" | grep -qE '|| |if \$|if !'; then + + # Check if the variable is used immediately after without validation + var_name=$(echo "$line_content" | sed -E 's/.*([a-zA-Z_][a-zA-Z0-9_]*)=.*/\1/' | head -1) + next_line=$(sed -n "$((line_num+1))p" "$file" 2>/dev/null) + + if [ -n "$var_name" ] && echo "$next_line" | grep -qE "^\s*if\s+|^\s*for.*\$${var_name}|${var_name}.*|"; then + if ! is_suppressed "$file" "$line_num" "no-err-check"; then + echo "HIGH|$file|$line_num|[NO-ERR-CHECK] Variable from command assignment used without exit code validation" + count_issue "HIGH" + ((count++)) + [ "$count" -ge 15 ] && break + fi + fi + fi +done < <(grep -rn '=\$\(.*\(mysql\|curl\|wget\)' "$TOOLKIT_PATH" --include="*.sh" 2>/dev/null) + +echo "Found: $count missing error checks" +echo "" +} >> "$REPORT" + +#============================================================================== +# CHECK 96: Uninitialized Variable Comparisons (HIGH) +#============================================================================== +show_progress 96 "Uninitialized variable comparisons" +{ +echo "## CHECK 96: Uninitialized Variable Comparisons" +echo "Severity: HIGH" +echo "Pattern: Variables compared without initialization or error handling" +echo "Impact: Silent failures when variable is unset (false positives/negatives)" +echo "" + +count=0 +# Look for comparisons where variable result could be empty +while IFS=: read -r file line_num line_content; do + # Look for: [ "$VAR" = "value" ] where VAR is assigned from a command that could fail + # Pattern: Assignment from command substitution with no error check + if echo "$line_content" | grep -qE 'VAR=\$\(.*\).*\[\s*"\$VAR'; then + # Variable assigned from subshell, then compared + var=$(echo "$line_content" | sed -E 's/.*([a-zA-Z_][a-zA-Z0-9_]*)=\$.*/\1/' | head -1) + if [ -n "$var" ] && ! echo "$line_content" | grep -qE '\$\{'"$var"':-'; then + if ! is_suppressed "$file" "$line_num" "uninit-var"; then + echo "HIGH|$file|$line_num|[UNINIT-VAR] Variable '\$$var' compared without checking if command succeeded" + count_issue "HIGH" + ((count++)) + [ "$count" -ge 15 ] && break + fi + fi + fi +done < <(grep -rn '\[\s*"\$.*=.*\$(' "$TOOLKIT_PATH" --include="*.sh" 2>/dev/null | grep -v 'VAR:-' | head -200) + +echo "Found: $count uninitialized variable comparisons" +echo "" +} >> "$REPORT" + +#============================================================================== +# CHECK 97: Variable Shadowing in Subshells (HIGH) +#============================================================================== +show_progress 97 "Variable shadowing in subshells/pipes" +{ +echo "## CHECK 97: Variable Shadowing in Subshells" +echo "Severity: HIGH" +echo "Pattern: Variables modified in pipes/subshells - changes lost after scope ends" +echo "Examples: count=0; cmd | while read; do count=$((count+1)); done (count stays 0)" +echo "" + +count=0 +while IFS=: read -r file line_num line_content; do + # Pattern 1: variable | while/for pattern + if echo "$line_content" | grep -qE '[a-zA-Z_][a-zA-Z0-9_]*\s*\|.*while|for.*\|.*while'; then + if ! is_suppressed "$file" "$line_num" "subshell-shadow"; then + echo "HIGH|$file|$line_num|[SUBSHELL-SHADOW] Variable may be shadowed by pipe/subshell (changes lost after loop)" + count_issue "HIGH" + ((count++)) + [ "$count" -ge 15 ] && break + fi + fi + + # Pattern 2: Assignment inside while/for loop from pipe + if echo "$line_content" | grep -qE 'done\s*<\s*<\s*\(|while.*<\s*<\s*\('; then + # Check if variables are modified in this loop + loop_content=$(sed -n "${line_num},/done/p" "$file" 2>/dev/null) + if echo "$loop_content" | grep -qE '[a-zA-Z_][a-zA-Z0-9_]*=.*\+\+|[a-zA-Z_][a-zA-Z0-9_]*=\$\(\('; then + if ! is_suppressed "$file" "$line_num" "subshell-shadow"; then + echo "HIGH|$file|$line_num|[SUBSHELL-SHADOW] Variable modified in process substitution (changes may be lost)" + count_issue "HIGH" + ((count++)) + [ "$count" -ge 15 ] && break + fi + fi + fi +done < <(grep -rn 'while\s\|for\s\|done\s*<\s*<' "$TOOLKIT_PATH" --include="*.sh" 2>/dev/null) + +echo "Found: $count variable shadowing issues" +echo "" +} >> "$REPORT" + +#============================================================================== +# CHECK 98: Array Access Without Bounds Check (HIGH) +#============================================================================== +show_progress 98 "Array access without bounds checking" +{ +echo "## CHECK 98: Array Access Without Bounds Checking" +echo "Severity: HIGH" +echo "Pattern: Direct array element access without verifying array is non-empty" +echo "Impact: Accesses undefined array indices, silent failures" +echo "" + +count=0 +# Only flag direct indexed access like ${arr[0]} or ${arr[$i]} with no bounds check +while IFS=: read -r file line_num line_content; do + # Pattern: Direct array index access (not array expansion) + if echo "$line_content" | grep -qE '\$\{[a-zA-Z_][a-zA-Z0-9_]*\[[0-9]|\$\{[a-zA-Z_][a-zA-Z0-9_]*\[\$'; then + # Extract array name and index + array_name=$(echo "$line_content" | sed -E 's/.*\$\{([a-zA-Z_][a-zA-Z0-9_]*)\[.*/\1/' | head -1) + + if [ -n "$array_name" ]; then + # Check if there's explicit initialization of this array in the file + arr_init=$(grep -n "^${array_name}=()\|^${array_name}=(" "$file" 2>/dev/null | wc -l) + + # If no explicit init and direct access with index, likely needs bounds check + if [ "$arr_init" -eq 0 ]; then + if ! is_suppressed "$file" "$line_num" "array-bounds"; then + echo "HIGH|$file|$line_num|[ARRAY-BOUNDS] Direct array index access without array initialization" + count_issue "HIGH" + ((count++)) + [ "$count" -ge 10 ] && break + fi + fi + fi + fi +done < <(grep -rn '\$\{[a-zA-Z_][a-zA-Z0-9_]*\[[0-9]\|\$\{[a-zA-Z_][a-zA-Z0-9_]*\[\$' "$TOOLKIT_PATH" --include="*.sh" 2>/dev/null) + +echo "Found: $count array access without bounds checks" +echo "" +} >> "$REPORT" + #============================================================================== # PERFORMANCE CHECKS (INFO level - not counted as issues) #==============================================================================