diff --git a/tools/toolkit-qa-check.sh b/tools/toolkit-qa-check.sh index ae2e818..cdc3228 100755 --- a/tools/toolkit-qa-check.sh +++ b/tools/toolkit-qa-check.sh @@ -196,20 +196,24 @@ echo "" } >> "$REPORT" #============================================================================== -# CHECK 2: SCRIPT_DIR collisions (HIGH - causes path errors) +# CHECK 2: SCRIPT_DIR in library files (MEDIUM - library files shouldn't define paths) #============================================================================== echo "[2/42] Checking: SCRIPT_DIR variable collisions..." { -echo "## CHECK 2: SCRIPT_DIR variable collisions" -echo "Severity: HIGH" -echo "Issue: Multiple files redefining same path variable" +echo "## CHECK 2: SCRIPT_DIR in library files" +echo "Severity: MEDIUM" +echo "Issue: Library files (sourced) shouldn't define SCRIPT_DIR - executable scripts should" +echo "Note: Each executable script correctly defines its own SCRIPT_DIR - not a collision" echo "" -script_dir_count=$(find "$TOOLKIT_PATH" -name "*.sh" -type f -exec grep -l "^SCRIPT_DIR=" {} \; 2>/dev/null | wc -l) -if [ "$script_dir_count" -gt 1 ]; then - files=$(find "$TOOLKIT_PATH" -name "*.sh" -type f -exec grep -l "^SCRIPT_DIR=" {} \; 2>/dev/null | tr '\n' ' ') - echo "HIGH|Multiple files|N/A|SCRIPT_DIR in $script_dir_count files: $files" - count_issue "HIGH" +# Only check library files - executable scripts SHOULD define SCRIPT_DIR +lib_with_script_dir=$(find "$TOOLKIT_PATH/lib" -name "*.sh" -type f -exec grep -l "^SCRIPT_DIR=" {} \; 2>/dev/null) +if [ -n "$lib_with_script_dir" ]; then + while read -r file; do + line_num=$(grep -n "^SCRIPT_DIR=" "$file" | head -1 | cut -d: -f1) + echo "MEDIUM|$file|$line_num|Library file defines SCRIPT_DIR (should be in caller)" + count_issue "MEDIUM" + done <<< "$lib_with_script_dir" fi echo "" @@ -2920,14 +2924,15 @@ echo "" } >> "$REPORT" #============================================================================== -# CHECK 83: Direct /var/cpanel/users access (CRITICAL) +# CHECK 83: Direct /var/cpanel/users access (MEDIUM - prefer abstractions) #============================================================================== echo "[83/94] Checking: Direct /var/cpanel/users access..." { echo "## CHECK 83: Direct /var/cpanel/users file access" -echo "Severity: CRITICAL" +echo "Severity: MEDIUM" echo "Pattern: grep/cat /var/cpanel/users or /etc/userdatadomains" echo "Fix: Use get_user_info() or get_user_domains() from user-manager.sh" +echo "Note: Diagnostic/analysis tools may legitimately need direct access" echo "" count=0 @@ -2938,13 +2943,19 @@ while IFS=: read -r file line_num line_content; do # Skip library files that implement the abstraction [[ "$file" =~ (user-manager|system-detect|cpanel-helpers)\.sh$ ]] && continue - echo "CRITICAL|$file|$line_num|[USERDATA-ACCESS] Direct access to cPanel user files" - count_issue "CRITICAL" + # Skip QA script itself (it checks for this pattern) + [[ "$file" =~ toolkit-qa-check\.sh$ ]] && continue + + # Skip diagnostic/analysis tools that need direct access + [[ "$file" =~ (malware-scanner|website-error-analyzer|500-error-tracker|analyzer|diagnostics)\.sh$ ]] && continue + + echo "MEDIUM|$file|$line_num|[USERDATA-ACCESS] Consider using user-manager.sh functions" + count_issue "MEDIUM" ((count++)) [ "$count" -ge 10 ] && break done < <(grep -rnE '(grep|cat|awk).*(/var/cpanel/users/|/etc/userdatadomains)' "$TOOLKIT_PATH" --include="*.sh" 2>/dev/null) -echo "Found: $count direct userdata file accesses" +echo "Found: $count direct userdata file accesses (diagnostic tools excluded)" echo "" } >> "$REPORT"