Problem/Motivation

We've tried using the Drush command provided in #2919464: Provide drush command for re-scanning existing files and have couple issues:

  • with a large amount of files to scan, we have around 200k, scanning could take a while and, in case of a antivirus service outage, there is no way to tell which files were skipped from scanning
  • by default scanner would report only the file name if the virus is detected, but this information is not complete for the case of scanning historical data
  • It does not do $this->scanner->isScannable() check so "Scannable schemes / stream wrappers" selection or hook_clamav_file_is_scannable implementations have no effect

Proposed resolution

Remaining tasks

code
review
commit

User interface changes

Additional option for the Drush command to scan only specified file IDs.

API changes

None/TBD

Issue fork clamav-3505409

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

rosk0 created an issue. See original summary.

rosk0 opened merge request !19

rosk0’s picture

rosk0
he/him
Primary language Russian
Location Wellington
commented
Assigned: rosk0 » Unassigned
Status: Active » Needs review

Manually tested locally with TCP/IP and executable modes - works as expected.

Not sure what GitLab doesn't like about my branch - I could merge it into the 2.x locally with no issues in fast-forward mode, also this is confirmed by the green tick mark ( if I understand this correctly ) next to the "Merge request must be rebased, because a fast-forward merge is not possible." check in the merge request overview.

patrickharris made their first commit to this issue’s fork.

almunnings’s picture

almunnings
Location Melbourne, 🇦🇺
commented

Is the namespace of this command incorrect?

With drush 12
drush status --debug

Results in 

 [debug] Could not instantiate Drupal\clamav\Drush\Commands\ClamavCommand: Too few arguments to function Drupal\Clamav\Drush\Commands\ClamavCommand::__construct(), 0 passed in /app/vendor/drush/drush/src/Runtime/ServiceManager.php on line 333 and exactly 3 expected [0.88 sec, 53.82 MB]

Suggest changing:

\Drupal\clamav\Drush\Commands\ClamavCommand
to
\Drupal\clamav\Commands\ClamavCommand

And moving the file accordingly

sokru’s picture

sokru commented
Issue summary: View changes
Status: Needs review » Needs work

Not sure if we should generate separate bug issue for missing isScannable check. The processScannerBatch code would need something like this: 

      $isScannable = $this->scanner->isScannable($file);

      if (!$isScannable) {
        $context['results']['unchecked']++;
        Drush::logger()->notice("File is not scannable: {$uri}");
        continue;
      }

Also as bigger architectural decision is if the scan results should be stored on some database table. On large sites it would be beneficial to split the scans on smaller chunks and retry the failed scans. Eg. storing the last scan date, would allow to only scan files older than X days.

sokru’s picture

sokru commented

#5 the namespace has case issue on https://git.drupalcode.org/project/clamav/-/blob/2.1.x/src/Drush/Command..., it should be namespace Drupal\clamav\Drush\Commands; instead of namespace Drupal\Clamav\Drush\Commands;.

sokru’s picture

sokru commented
Issue summary: View changes