Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the core scanning functionality to avoid timeout errors by implementing a more efficient batch processing system and removing the dependency on the YARA framework in favor of advanced malware detection algorithms.
Key Changes:
- Replacement of YARA Integration: Removed YARA framework dependency and replaced with AMWScanner for improved performance and reduced timeout issues
- Batch Processing Implementation: Added memory-efficient batch scanning with real-time progress tracking to handle large file sets without timeout
- Enhanced Error Handling: Improved error handling, logging, and user feedback throughout the scanning process
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| wsp-malware-scanner.php | Updated plugin description to reflect new scanning technology |
| src/rules/malware_rules.yar | Removed YARA rules file as part of framework migration |
| src/Init.php | Removed YaraIntegration service registration and instantiation |
| src/Core/YaraIntegration.php | Removed entire YARA integration class |
| src/Core/Scanner.php | Complete rewrite with batch processing, AMWScanner integration, and memory management |
| src/Core/Updater.php | Enhanced with better error handling, caching, and retry mechanisms |
| src/Core/Logger.php | Improved with better error handling, log rotation, and fallback directories |
| src/Core/EmailService.php | Enhanced with multi-email support and better error handling |
| src/Core/AutoScanner.php | Added better error handling and validation |
| src/Core/Activator.php | Enhanced with comprehensive default options and table structure updates |
| src/Core/Deactivator.php | Improved with proper cleanup and error handling |
| src/Admin/views/scanner.php | Updated UI for batch scanning with real-time progress |
| src/Admin/views/main.php | Enhanced dashboard with better error handling and debug info |
| src/Admin/assets/js/admin-scripts.js | Complete rewrite for batch scanning with real-time progress updates |
| src/Admin/assets/css/admin-styles.css | Updated styles for new batch scanning interface |
| src/Admin/AdminPage.php | Updated asset versions for cache busting |
| composer.json | Updated description to reflect new scanning technology |
| README.md | Updated documentation to reflect YARA framework removal |
Comments suppressed due to low confidence (1)
|
|
||
| // CRITICAL FIX: Use proper AMWScanner methods instead of static properties | ||
| // This is the key fix that makes malware detection work properly | ||
| $scanner->setScanAll(true); // Enable scanning all file types |
There was a problem hiding this comment.
The comment indicates enabling scanning of all file types, but this could potentially impact performance. Consider adding documentation about the performance implications and whether this setting should be configurable.
| $scanner->setScanAll(true); // Enable scanning all file types | |
| // Make scanning all file types configurable to balance performance and detection coverage | |
| $scan_all_files = get_option('malware_scanner_scan_all', true); // Default: true | |
| $scanner->setScanAll($scan_all_files); | |
| if ($scan_all_files) { | |
| Logger::info('AMWScanner: Scanning all file types enabled. This may impact performance on large file systems.'); | |
| } else { | |
| Logger::info('AMWScanner: Scanning all file types disabled. Some files may be skipped.'); | |
| } |
| $plugin_base_path = str_replace(WP_PLUGIN_DIR . '/', '', dirname(__DIR__, 2)); | ||
| $ignore_folders[] = 'wp-content/plugins/' . $plugin_base_path . '/vendor'; | ||
| $ignore_folders[] = 'wp-content/plugins/' . $plugin_base_path . '/logs'; | ||
| $ignore_folders[] = 'wp-content/plugins/' . $plugin_base_path . '/src'; |
There was a problem hiding this comment.
Hard-coding ignore paths for the plugin's own directories could be a security concern if the plugin base path is manipulated. Consider validating the plugin base path before using it in ignore patterns.
| $plugin_base_path = str_replace(WP_PLUGIN_DIR . '/', '', dirname(__DIR__, 2)); | |
| $ignore_folders[] = 'wp-content/plugins/' . $plugin_base_path . '/vendor'; | |
| $ignore_folders[] = 'wp-content/plugins/' . $plugin_base_path . '/logs'; | |
| $ignore_folders[] = 'wp-content/plugins/' . $plugin_base_path . '/src'; | |
| $plugin_base_path = realpath(dirname(__DIR__, 2)); | |
| // Validate the plugin base path | |
| if ($plugin_base_path === false || strpos($plugin_base_path, realpath(WP_PLUGIN_DIR)) !== 0) { | |
| Logger::error('Invalid plugin base path detected: ' . $plugin_base_path); | |
| wp_send_json_error(['message' => 'Invalid plugin base path.']); | |
| return; | |
| } | |
| // Convert the validated base path to a relative path | |
| $relative_plugin_path = str_replace(WP_PLUGIN_DIR . '/', '', $plugin_base_path); | |
| $ignore_folders[] = 'wp-content/plugins/' . $relative_plugin_path . '/vendor'; | |
| $ignore_folders[] = 'wp-content/plugins/' . $relative_plugin_path . '/logs'; | |
| $ignore_folders[] = 'wp-content/plugins/' . $relative_plugin_path . '/src'; |
| // Option 1: Try WordPress upload directory (preferred) | ||
| $upload_dir = wp_upload_dir(); | ||
| if ($upload_dir && !isset($upload_dir['error']) && !empty($upload_dir['basedir'])) { | ||
| $log_dir = $upload_dir['basedir'] . '/malware-scanner-logs'; | ||
| if (self::ensure_directory_exists($log_dir)) { | ||
| return $log_dir; | ||
| } | ||
| } | ||
|
|
||
| // Option 2: Try plugin directory | ||
| $plugin_dir = dirname(__DIR__, 2) . '/logs'; | ||
| if (self::ensure_directory_exists($plugin_dir)) { | ||
| return $plugin_dir; | ||
| } | ||
|
|
||
| // Option 3: Try WordPress content directory | ||
| if (defined('WP_CONTENT_DIR')) { | ||
| $content_log_dir = WP_CONTENT_DIR . '/malware-scanner-logs'; | ||
| if (self::ensure_directory_exists($content_log_dir)) { | ||
| return $content_log_dir; | ||
| } | ||
| } | ||
|
|
||
| // Option 4: Try temporary directory | ||
| $temp_dir = sys_get_temp_dir() . '/malware-scanner-logs'; | ||
| if (self::ensure_directory_exists($temp_dir)) { | ||
| return $temp_dir; | ||
| } | ||
|
|
||
| // Option 5: Last resort - try current directory | ||
| $current_dir = __DIR__ . '/logs'; | ||
| if (self::ensure_directory_exists($current_dir)) { | ||
| return $current_dir; | ||
| } |
There was a problem hiding this comment.
[nitpick] The multiple fallback directory options in get_log_directory() create complex logic that could be simplified. Consider extracting the directory validation logic into a separate method to improve readability.
| // Option 1: Try WordPress upload directory (preferred) | |
| $upload_dir = wp_upload_dir(); | |
| if ($upload_dir && !isset($upload_dir['error']) && !empty($upload_dir['basedir'])) { | |
| $log_dir = $upload_dir['basedir'] . '/malware-scanner-logs'; | |
| if (self::ensure_directory_exists($log_dir)) { | |
| return $log_dir; | |
| } | |
| } | |
| // Option 2: Try plugin directory | |
| $plugin_dir = dirname(__DIR__, 2) . '/logs'; | |
| if (self::ensure_directory_exists($plugin_dir)) { | |
| return $plugin_dir; | |
| } | |
| // Option 3: Try WordPress content directory | |
| if (defined('WP_CONTENT_DIR')) { | |
| $content_log_dir = WP_CONTENT_DIR . '/malware-scanner-logs'; | |
| if (self::ensure_directory_exists($content_log_dir)) { | |
| return $content_log_dir; | |
| } | |
| } | |
| // Option 4: Try temporary directory | |
| $temp_dir = sys_get_temp_dir() . '/malware-scanner-logs'; | |
| if (self::ensure_directory_exists($temp_dir)) { | |
| return $temp_dir; | |
| } | |
| // Option 5: Last resort - try current directory | |
| $current_dir = __DIR__ . '/logs'; | |
| if (self::ensure_directory_exists($current_dir)) { | |
| return $current_dir; | |
| } | |
| // Helper method to validate and ensure directory exists | |
| private static function validate_and_ensure_directory($path) { | |
| if (self::ensure_directory_exists($path)) { | |
| return $path; | |
| } | |
| return false; | |
| } | |
| // Option 1: Try WordPress upload directory (preferred) | |
| $upload_dir = wp_upload_dir(); | |
| if ($upload_dir && !isset($upload_dir['error']) && !empty($upload_dir['basedir'])) { | |
| $log_dir = $upload_dir['basedir'] . '/malware-scanner-logs'; | |
| return self::validate_and_ensure_directory($log_dir); | |
| } | |
| // Option 2: Try plugin directory | |
| $plugin_dir = dirname(__DIR__, 2) . '/logs'; | |
| return self::validate_and_ensure_directory($plugin_dir); | |
| // Option 3: Try WordPress content directory | |
| if (defined('WP_CONTENT_DIR')) { | |
| $content_log_dir = WP_CONTENT_DIR . '/malware-scanner-logs'; | |
| return self::validate_and_ensure_directory($content_log_dir); | |
| } | |
| // Option 4: Try temporary directory | |
| $temp_dir = sys_get_temp_dir() . '/malware-scanner-logs'; | |
| return self::validate_and_ensure_directory($temp_dir); | |
| // Option 5: Last resort - try current directory | |
| $current_dir = __DIR__ . '/logs'; | |
| return self::validate_and_ensure_directory($current_dir); |
| function handleScanError(errorMessage) { | ||
| const progressStatus = document.getElementById('progress-status'); | ||
| const progressBar = document.getElementById('progress-bar-fill'); | ||
| const startButton = document.getElementById('start-scan'); | ||
|
|
||
| // Check for common error types and provide better user messages | ||
| let userFriendlyMessage = errorMessage; | ||
|
|
||
| if (errorMessage.includes('memory') || errorMessage.includes('Memory') || errorMessage.includes('exhausted')) { | ||
| userFriendlyMessage = '🔧 Memory limit reached. The scanner will automatically optimize memory usage on retry. Please try again.'; | ||
| } else if (errorMessage.includes('timeout') || errorMessage.includes('execution time')) { | ||
| userFriendlyMessage = '⏰ Scan timeout prevented. The scanner will process files more efficiently on retry.'; | ||
| } else if (errorMessage.includes('permission') || errorMessage.includes('Permission')) { | ||
| userFriendlyMessage = '🔒 File permission error. Please check file permissions and try again.'; | ||
| } else if (errorMessage.includes('network') || errorMessage.includes('Network')) { | ||
| userFriendlyMessage = '🌐 Network connection issue. Please check your connection and try again.'; | ||
| } | ||
|
|
There was a problem hiding this comment.
The string-based error message matching could be unreliable across different locales and error message formats. Consider using more structured error handling with error codes or types instead of string matching.
| function handleScanError(errorMessage) { | |
| const progressStatus = document.getElementById('progress-status'); | |
| const progressBar = document.getElementById('progress-bar-fill'); | |
| const startButton = document.getElementById('start-scan'); | |
| // Check for common error types and provide better user messages | |
| let userFriendlyMessage = errorMessage; | |
| if (errorMessage.includes('memory') || errorMessage.includes('Memory') || errorMessage.includes('exhausted')) { | |
| userFriendlyMessage = '🔧 Memory limit reached. The scanner will automatically optimize memory usage on retry. Please try again.'; | |
| } else if (errorMessage.includes('timeout') || errorMessage.includes('execution time')) { | |
| userFriendlyMessage = '⏰ Scan timeout prevented. The scanner will process files more efficiently on retry.'; | |
| } else if (errorMessage.includes('permission') || errorMessage.includes('Permission')) { | |
| userFriendlyMessage = '🔒 File permission error. Please check file permissions and try again.'; | |
| } else if (errorMessage.includes('network') || errorMessage.includes('Network')) { | |
| userFriendlyMessage = '🌐 Network connection issue. Please check your connection and try again.'; | |
| } | |
| function handleScanError(errorCode, errorMessage) { | |
| const progressStatus = document.getElementById('progress-status'); | |
| const progressBar = document.getElementById('progress-bar-fill'); | |
| const startButton = document.getElementById('start-scan'); | |
| // Map error codes to user-friendly messages | |
| const errorMessages = { | |
| 'MEMORY_LIMIT': '🔧 Memory limit reached. The scanner will automatically optimize memory usage on retry. Please try again.', | |
| 'TIMEOUT': '⏰ Scan timeout prevented. The scanner will process files more efficiently on retry.', | |
| 'PERMISSION_DENIED': '🔒 File permission error. Please check file permissions and try again.', | |
| 'NETWORK_ERROR': '🌐 Network connection issue. Please check your connection and try again.', | |
| }; | |
| const userFriendlyMessage = errorMessages[errorCode] || 'An unexpected error occurred. Please try again.'; |
|
Please resolve the Co-Pilot suggestion |
Update core scanning to avoid timeout error