New. BinaryCheck. Init module.#593
Conversation
# Conflicts: # inc/spbc-backups.php
alexandergull
left a comment
There was a problem hiding this comment.
https://app.doboard.com/1/task/43500 - task is not ready, request a new review since it is.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “Binary analysis” scanning stage to detect and surface potentially suspicious binary/executable files, integrates it into the scanner pipeline/UI, and adds safer handling for viewing binary files in scan results.
Changes:
- Added a new scanner stage (
binary_analysis) with supporting storage/reporting and a binary-check module that inspects file headers. - Updated scanner queries and actions to exclude binary-tagged files from cloud send / cure / heuristic/signature flows.
- Added UI/settings wiring for enabling the stage and rendering binary files as a hex dump when viewing file contents.
Reviewed changes
Copilot reviewed 12 out of 15 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/CleantalkSP/SpbctWP/State.php | Adds default setting for enabling binary analysis. |
| lib/CleantalkSP/SpbctWP/SpbcEnqueue.php | Exposes new stage flags/translations to the frontend. |
| lib/CleantalkSP/SpbctWP/Scanner/Surface.php | Includes extensionless files in scan discovery (binary candidates). |
| lib/CleantalkSP/SpbctWP/Scanner/Stages/CureStage.php | Excludes binary-tagged files from curing selection. |
| lib/CleantalkSP/SpbctWP/Scanner/Services/SendFileToCloudService.php | Prevents sending binary-tagged files to cloud analysis. |
| lib/CleantalkSP/SpbctWP/Scanner/ScanningStagesModule/Stages/BinaryAnalysis.php | New stage data object for logging/progress. |
| lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php | Adds the binary_analysis stage and integrates module execution + SQL filtering. |
| lib/CleantalkSP/SpbctWP/Scanner/ScannerActions/FileSystemActions.php | Adds hex-dump rendering when viewing binary files. |
| lib/CleantalkSP/SpbctWP/Scanner/ScannerActions/CloudAnalysisActions.php | Excludes binary-tagged files from bulk cloud-send selection. |
| lib/CleantalkSP/SpbctWP/Scanner/BinaryCheckModule/BinaryCheckModule.php | New module performing header-based binary detection and DB status updates. |
| js/src/spbc-scanner-plugin.js | Adds binary stage handling in the scanner frontend logic. |
| js/spbc-scanner-plugin.min.js | Built/minified output reflecting the new stage. |
| js/spbc-scanner-plugin.min.js.map | Source map updated for the new stage. |
| inc/spbc-settings.php | Adds the settings field and UI stage label; adjusts actions for binary files. |
Comments suppressed due to low confidence (1)
lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php:1035
sourceis being set to an empty string for non-binary files. This breaks logic that relies onsource IS NULLto identify unknown files (e.g., UnknownRepository), and also makes laterisset($file_info['source'])checks treat the source as “known”. Store NULL for non-binary files and insert SQL NULL (not an empty string) so existing unknown-file workflows keep working.
// if extension is not set, set 'source' to 'binary'
$file['source'] = ! $ext ? 'BINARY' : '';
if ( ! spbc_check_ascii($file['path']) ) {
$sql_query__values_non_ascii[] = '(\'' . implode('\',\'', $file) . '\')';
} else {
$sql_query__values[] = '(\'' . implode('\',\'', $file) . '\')';
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( is_file($path) && !is_link($path) ) { | ||
| $currentFileExtension = pathinfo($path, PATHINFO_EXTENSION); | ||
|
|
||
| if ($currentFileExtension === '') { | ||
| $this->output_counter_files++; | ||
| $this->output_files[]['path'] = $path; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
I think that's wrong.
There was a problem hiding this comment.
What is this logic for? If you want to count no-extension files, modify the logic before extension check.
if ($currentFileExtension !== "") {
if ( $this->ext_except || $this->ext ) ...
\further extension check
}
There was a problem hiding this comment.
Note, this is not the same logic as just counting files. If you skip the fie the way it is now, next iterator logic may be broken.
| public function run($amount = 4, $offset = 0) | ||
| { | ||
| $this->batch_count = 0; | ||
| $this->scanned_count = 0; | ||
| $this->statuses = array(); | ||
|
|
||
| $binary_files = $this->getBinaryFiles($amount, $offset); | ||
| $this->batch_count = count($binary_files); | ||
|
|
||
| $this->analyzeBinaryFiles($binary_files); | ||
|
|
||
| return array( | ||
| 'success' => true, | ||
| 'batch_count' => $this->batch_count, | ||
| 'scanned_count' => $this->scanned_count, | ||
| 'statuses' => $this->statuses, | ||
| ); |
There was a problem hiding this comment.
So please provide a new task to cover the Binary analysis with unit-test.
There was a problem hiding this comment.
| if ( is_file($path) && !is_link($path) ) { | ||
| $currentFileExtension = pathinfo($path, PATHINFO_EXTENSION); | ||
|
|
||
| if ($currentFileExtension === '') { | ||
| $this->output_counter_files++; | ||
| $this->output_files[]['path'] = $path; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Note, this is not the same logic as just counting files. If you skip the fie the way it is now, next iterator logic may be broken.
| public function run($amount = 4, $offset = 0) | ||
| { | ||
| $this->batch_count = 0; | ||
| $this->scanned_count = 0; | ||
| $this->statuses = array(); | ||
|
|
||
| $binary_files = $this->getBinaryFiles($amount, $offset); | ||
| $this->batch_count = count($binary_files); | ||
|
|
||
| $this->analyzeBinaryFiles($binary_files); | ||
|
|
||
| return array( | ||
| 'success' => true, | ||
| 'batch_count' => $this->batch_count, | ||
| 'scanned_count' => $this->scanned_count, | ||
| 'statuses' => $this->statuses, | ||
| ); |
There was a problem hiding this comment.
So please provide a new task to cover the Binary analysis with unit-test.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ck-init # Conflicts: # inc/spbc-settings.php
task https://app.doboard.com/1/task/29314