diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index 3d7687f1..e380d77d 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -10,10 +10,10 @@ env: jobs: package: name: Package nightly release - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Setup krankler run: | wget https://github.com/ChristophWurst/krankerl/releases/download/v0.13.3/krankerl @@ -21,7 +21,7 @@ jobs: - name: Package app run: | ./krankerl package - - uses: actions/upload-artifact@v2 + - uses: actions/upload-artifact@v4 with: name: ${{ env.APP_NAME }}.tar.gz path: build/artifacts/${{ env.APP_NAME }}.tar.gz diff --git a/lib/AppConfig.php b/lib/AppConfig.php index c99e5327..1dcc6ddf 100644 --- a/lib/AppConfig.php +++ b/lib/AppConfig.php @@ -65,6 +65,7 @@ class AppConfig { 'av_icap_chunk_size' => '1048576', 'av_icap_connect_timeout' => '5', 'av_scan_first_bytes' => -1, + 'av_block_unscannable' => false, ]; /** @@ -94,6 +95,14 @@ public function setAvIcapTls(bool $enable): void { $this->setAppValue('av_icap_tls', $enable ? '1' : '0'); } + public function getAvBlockUnscannable(): bool { + return (bool)$this->getAppValue('av_block_unscannable'); + } + + public function setAvBlockUnscannable(bool $block): void { + $this->setAppValue('av_block_unscannable', $block ? '1' : '0'); + } + /** * Get full commandline * diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index b38a31f6..79eb74b9 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -100,6 +100,8 @@ function (string $mountPoint, IStorage $storage) { $activityManager = $container->get(IManager::class); $eventDispatcher = $container->get(IEventDispatcher::class); $appManager = $container->get(IAppManager::class); + /** @var AppConfig $appConfig */ + $appConfig = $container->get(AppConfig::class); return new AvirWrapper([ 'storage' => $storage, 'scannerFactory' => $scannerFactory, @@ -110,6 +112,7 @@ function (string $mountPoint, IStorage $storage) { 'eventDispatcher' => $eventDispatcher, 'trashEnabled' => $appManager->isEnabledForUser('files_trashbin'), 'mount_point' => $mountPoint, + 'block_unscannable' => $appConfig->getAvBlockUnscannable(), ]); }, 1 diff --git a/lib/AvirWrapper.php b/lib/AvirWrapper.php index 75316d14..a9d46b8d 100644 --- a/lib/AvirWrapper.php +++ b/lib/AvirWrapper.php @@ -32,6 +32,7 @@ class AvirWrapper extends Wrapper { private bool $shouldScan = true; private bool $trashEnabled; private string $mountPoint; + private bool $blockUnscannable = false; /** * @param array $parameters @@ -45,6 +46,7 @@ public function __construct($parameters) { $this->isHomeStorage = $parameters['isHomeStorage']; $this->trashEnabled = $parameters['trashEnabled']; $this->mountPoint = $parameters['mount_point']; + $this->blockUnscannable = $parameters['block_unscannable']; /** @var IEventDispatcher $eventDispatcher */ $eventDispatcher = $parameters['eventDispatcher']; @@ -107,6 +109,9 @@ function () use ($scanner, $path) { if ($status->getNumericStatus() === Status::SCANRESULT_INFECTED) { $this->handleInfected($path, $status); } + if ($this->blockUnscannable && $status->getNumericStatus() === Status::SCANRESULT_UNSCANNABLE) { + $this->handleInfected($path, $status); + } } ); } catch (\Exception $e) { diff --git a/lib/BackgroundJob/BackgroundScanner.php b/lib/BackgroundJob/BackgroundScanner.php index 54dd62fb..4490bcd3 100644 --- a/lib/BackgroundJob/BackgroundScanner.php +++ b/lib/BackgroundJob/BackgroundScanner.php @@ -212,7 +212,7 @@ public function getUnscannedFiles() { ->andWhere($query->expr()->neq('mimetype', $query->createNamedParameter($dirMimeTypeId))) ->andWhere($query->expr()->orX( $query->expr()->like('path', $query->createNamedParameter('files/%')), - $query->expr()->notLike('s.id', $query->createNamedParameter("home::%")) + $query->expr()->notLike('s.id', $query->createNamedParameter('home::%')) )) ->andWhere($this->getSizeLimitExpression($query)) ->setMaxResults($this->getBatchSize() * 10); diff --git a/lib/Command/BackgroundScan.php b/lib/Command/BackgroundScan.php index aa216c6c..4ad696fb 100644 --- a/lib/Command/BackgroundScan.php +++ b/lib/Command/BackgroundScan.php @@ -35,7 +35,7 @@ protected function configure() { $this ->setName('files_antivirus:background-scan') ->setDescription('Run the background scan') - ->addOption('max', 'm', InputOption::VALUE_REQUIRED, "Maximum number of files to process"); + ->addOption('max', 'm', InputOption::VALUE_REQUIRED, 'Maximum number of files to process'); } protected function execute(InputInterface $input, OutputInterface $output): int { @@ -61,7 +61,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln("scanned $count files"); if ($count === $max) { - $output->writeln(" there might still be unscanned files remaining"); + $output->writeln(' there might still be unscanned files remaining'); } return 0; diff --git a/lib/Command/Mark.php b/lib/Command/Mark.php index 8ad488fe..c262bc0d 100644 --- a/lib/Command/Mark.php +++ b/lib/Command/Mark.php @@ -37,9 +37,9 @@ protected function configure() { $this ->setName('files_antivirus:mark') ->setDescription('Mark a file as scanned or unscanned') - ->addOption('forever', 'f', InputOption::VALUE_NONE, "When marking a file as scanned, set it to never rescan the file in the future") - ->addArgument('file', InputArgument::REQUIRED, "Path of the file to mark") - ->addArgument('mode', InputArgument::REQUIRED, "Either scanned or unscanned"); + ->addOption('forever', 'f', InputOption::VALUE_NONE, 'When marking a file as scanned, set it to never rescan the file in the future') + ->addArgument('file', InputArgument::REQUIRED, 'Path of the file to mark') + ->addArgument('mode', InputArgument::REQUIRED, 'Either scanned or unscanned'); } protected function execute(InputInterface $input, OutputInterface $output): int { diff --git a/lib/Command/Scan.php b/lib/Command/Scan.php index b6150bd9..2c2c76a0 100644 --- a/lib/Command/Scan.php +++ b/lib/Command/Scan.php @@ -41,8 +41,8 @@ protected function configure() { $this ->setName('files_antivirus:scan') ->setDescription('Scan a file') - ->addArgument('file', InputArgument::REQUIRED, "Path of the file to scan") - ->addOption('debug', null, InputOption::VALUE_NONE, "Enable debug output for supported backends"); + ->addArgument('file', InputArgument::REQUIRED, 'Path of the file to scan') + ->addOption('debug', null, InputOption::VALUE_NONE, 'Enable debug output for supported backends'); } protected function execute(InputInterface $input, OutputInterface $output): int { @@ -73,18 +73,22 @@ protected function execute(InputInterface $input, OutputInterface $output): int $exit = 2; break; case \OCA\Files_Antivirus\Status::SCANRESULT_CLEAN: - $status = "is clean"; + $status = 'is clean'; $exit = 0; break; case \OCA\Files_Antivirus\Status::SCANRESULT_INFECTED: - $status = "is infected"; + $status = 'is infected'; $exit = 1; break; + case \OCA\Files_Antivirus\Status::SCANRESULT_UNSCANNABLE: + $status = 'is not scannable'; + $exit = 2; + break; } if ($result->getDetails()) { - $details = ": " . $result->getDetails(); + $details = ': ' . $result->getDetails(); } else { - $details = ""; + $details = ''; } $output->writeln("$path $status$details"); diff --git a/lib/Command/Status.php b/lib/Command/Status.php index 4c15f83b..c991c116 100644 --- a/lib/Command/Status.php +++ b/lib/Command/Status.php @@ -41,15 +41,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $unscanned = $this->backgroundScanner->getUnscannedFiles(); - $count = $this->processFiles($unscanned, $output, $verbose, "is unscanned"); + $count = $this->processFiles($unscanned, $output, $verbose, 'is unscanned'); $output->writeln("$count unscanned files"); $rescan = $this->backgroundScanner->getToRescanFiles(); - $count = $this->processFiles($rescan, $output, $verbose, "is scheduled for re-scan"); + $count = $this->processFiles($rescan, $output, $verbose, 'is scheduled for re-scan'); $output->writeln("$count files scheduled for re-scan"); $outdated = $this->backgroundScanner->getOutdatedFiles(); - $count = $this->processFiles($outdated, $output, $verbose, "has been updated"); + $count = $this->processFiles($outdated, $output, $verbose, 'has been updated'); $output->writeln("$count have been updated since the last scan"); return 0; diff --git a/lib/Command/Test.php b/lib/Command/Test.php index 152d63ae..e034ff89 100644 --- a/lib/Command/Test.php +++ b/lib/Command/Test.php @@ -40,33 +40,33 @@ protected function configure() { $this ->setName('files_antivirus:test') ->setDescription('Test the availability of the configured scanner') - ->addOption('debug', null, InputOption::VALUE_NONE, "Enable debug output for supported backends"); + ->addOption('debug', null, InputOption::VALUE_NONE, 'Enable debug output for supported backends'); } protected function execute(InputInterface $input, OutputInterface $output): int { - $output->write("Scanning regular text: "); + $output->write('Scanning regular text: '); $scanner = $this->scannerFactory->getScanner('/foo.txt'); if ($input->getOption('debug')) { - $output->writeln(""); + $output->writeln(''); $scanner->setDebugCallback(function ($content) use ($output) { $output->writeln($content); }); } - $result = $scanner->scanString("dummy scan content"); + $result = $scanner->scanString('dummy scan content'); if ($result->getNumericStatus() === Status::SCANRESULT_INFECTED) { $details = $result->getDetails(); $output->writeln("❌ $details"); return 1; } elseif ($result->getNumericStatus() === Status::SCANRESULT_UNCHECKED) { - $output->writeln("- file not scanned or scan still pending"); + $output->writeln('- file not scanned or scan still pending'); } else { - $output->writeln("✓"); + $output->writeln('✓'); } - $output->write("Scanning EICAR test file: "); + $output->write('Scanning EICAR test file: '); $scanner = $this->scannerFactory->getScanner('/test-virus-eicar.txt'); if ($input->getOption('debug')) { - $output->writeln(""); + $output->writeln(''); $scanner->setDebugCallback(function ($content) use ($output) { $output->writeln($content); }); @@ -78,17 +78,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln("❌ file not detected $details"); return 1; } elseif ($result->getNumericStatus() === Status::SCANRESULT_UNCHECKED) { - $output->writeln("- file not scanned or scan still pending"); + $output->writeln('- file not scanned or scan still pending'); + } elseif ($result->getNumericStatus() === Status::SCANRESULT_UNSCANNABLE) { + $output->writeln('- file could not be scanned'); } else { - $output->writeln("✓"); + $output->writeln('✓'); } // send a modified version of the EICAR because some scanners don't hold the scan request // by default for files that haven't been seen before. - $output->write("Scanning modified EICAR test file: "); + $output->write('Scanning modified EICAR test file: '); $scanner = $this->scannerFactory->getScanner('/test-virus-eicar-modified.txt'); if ($input->getOption('debug')) { - $output->writeln(""); + $output->writeln(''); $scanner->setDebugCallback(function ($content) use ($output) { $output->writeln($content); }); @@ -99,9 +101,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln("❌ file not detected $details"); return 1; } elseif ($result->getNumericStatus() === Status::SCANRESULT_UNCHECKED) { - $output->writeln("- file not scanned or scan still pending"); + $output->writeln('- file not scanned or scan still pending'); + } elseif ($result->getNumericStatus() === Status::SCANRESULT_UNSCANNABLE) { + $output->writeln('- file could not be scanned'); } else { - $output->writeln("✓"); + $output->writeln('✓'); } return 0; diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index abc04a48..2683087f 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -52,6 +52,7 @@ public function __construct($appName, IRequest $request, AppConfig $appconfig, I * @param int $avScanFirstBytes - scan size limit * @param string $avIcapMode * @param bool $avIcapTls + * @param bool $avBlockUnscannable * @return JSONResponse */ public function save( @@ -68,7 +69,8 @@ public function save( $avIcapMode, $avIcapRequestService, $avIcapResponseHeader, - $avIcapTls + $avIcapTls, + $avBlockUnscannable ) { $this->settings->setAvMode($avMode); $this->settings->setAvSocket($avSocket); @@ -84,10 +86,11 @@ public function save( $this->settings->setAvIcapRequestService($avIcapRequestService); $this->settings->setAvIcapResponseHeader($avIcapResponseHeader); $this->settings->setAvIcapTls((bool)$avIcapTls); + $this->settings->setAvBlockUnscannable((bool)$avBlockUnscannable); try { $scanner = $this->scannerFactory->getScanner('/self-test.txt'); - $result = $scanner->scanString("dummy scan content"); + $result = $scanner->scanString('dummy scan content'); $success = $result->getNumericStatus() == Status::SCANRESULT_CLEAN; $message = $success ? $this->l10n->t('Saved') : 'unexpected scan results for test content'; } catch (\Exception $e) { diff --git a/lib/Db/Rule.php b/lib/Db/Rule.php index 9fcad298..e1c1d630 100644 --- a/lib/Db/Rule.php +++ b/lib/Db/Rule.php @@ -43,35 +43,35 @@ class Rule extends Entity implements JsonSerializable { /** * * @var int statusType - RULE_TYPE_CODE or RULE_TYPE_MATCH defines whether - * rule should be checked by the shell exit code or regexp + * rule should be checked by the shell exit code or regexp */ protected $statusType; /** * * @var int result - shell exit code for rules - * of the type RULE_TYPE_CODE, 0 otherwise + * of the type RULE_TYPE_CODE, 0 otherwise */ protected $result; /** * * @var string match - regexp to match for rules - * of the type RULE_TYPE_MATCH, '' otherwise + * of the type RULE_TYPE_MATCH, '' otherwise */ protected $match; /** * * @var string description - shell exit code meaning for rules - * of the type RULE_TYPE_CODE, '' otherwise + * of the type RULE_TYPE_CODE, '' otherwise */ protected $description; /** * * @var int status - file check status. SCANRESULT_UNCHECKED, SCANRESULT_INFECTED, - * SCANRESULT_CLEAN are matching Unknown, Infected and Clean files accordingly. + * SCANRESULT_CLEAN are matching Unknown, Infected and Clean files accordingly. */ protected $status; diff --git a/lib/ICAP/ICAPRequest.php b/lib/ICAP/ICAPRequest.php index 69a1aba1..9bf8ae06 100644 --- a/lib/ICAP/ICAPRequest.php +++ b/lib/ICAP/ICAPRequest.php @@ -89,7 +89,7 @@ public function __construct( } if ($this->responseCallback) { - ($this->responseCallback)("ICAP Request headers:"); + ($this->responseCallback)('ICAP Request headers:'); ($this->responseCallback)($request); } @@ -107,7 +107,7 @@ public function finish(): IcapResponse { if ($this->responseCallback) { $response = stream_get_contents($this->stream); - ($this->responseCallback)("ICAP Response:"); + ($this->responseCallback)('ICAP Response:'); ($this->responseCallback)($response); $stream = fopen('php://temp', 'r+'); fwrite($stream, $response); diff --git a/lib/ICAP/ResponseParser.php b/lib/ICAP/ResponseParser.php index 5e2c3d95..7d3a3aa0 100644 --- a/lib/ICAP/ResponseParser.php +++ b/lib/ICAP/ResponseParser.php @@ -40,10 +40,10 @@ public function read_response($stream): IcapResponse { private function readIcapStatusLine($stream): IcapResponseStatus { $rawHeader = \fgets($stream); if (!$rawHeader) { - throw new RuntimeException("Empty ICAP response"); + throw new RuntimeException('Empty ICAP response'); } $icapHeader = \trim($rawHeader); - $numValues = \sscanf($icapHeader, "ICAP/%d.%d %d %s", $v1, $v2, $code, $status); + $numValues = \sscanf($icapHeader, 'ICAP/%d.%d %d %s', $v1, $v2, $code, $status); if ($numValues !== 4) { throw new RuntimeException("Unknown ICAP response: \"$icapHeader\""); } @@ -52,9 +52,9 @@ private function readIcapStatusLine($stream): IcapResponseStatus { private function parseEncapsulated(string $headerValue): array { $result = []; - $encapsulatedParts = \explode(",", $headerValue); + $encapsulatedParts = \explode(',', $headerValue); foreach ($encapsulatedParts as $encapsulatedPart) { - $pieces = \explode("=", \trim($encapsulatedPart)); + $pieces = \explode('=', \trim($encapsulatedPart)); $result[$pieces[0]] = (int)$pieces[1]; } return $result; @@ -85,7 +85,7 @@ private function readHeaders($stream): array { $headers = []; while (($headerString = \fgets($stream)) !== false) { $trimmedHeaderString = \trim($headerString); - if ($trimmedHeaderString === "") { + if ($trimmedHeaderString === '') { break; } [$headerName, $headerValue] = $this->parseHeader($trimmedHeaderString); diff --git a/lib/Scanner/ExternalClam.php b/lib/Scanner/ExternalClam.php index 738609b3..d919c44c 100644 --- a/lib/Scanner/ExternalClam.php +++ b/lib/Scanner/ExternalClam.php @@ -69,7 +69,7 @@ protected function shutdownScanner() { if ($info['timed_out']) { $this->status->setNumericStatus(Status::SCANRESULT_UNCHECKED); - $this->status->setDetails("Socket timed out while scanning"); + $this->status->setDetails('Socket timed out while scanning'); } else { $this->status->parseResponse($response); } diff --git a/lib/Scanner/ExternalKaspersky.php b/lib/Scanner/ExternalKaspersky.php index 84888111..35e87317 100644 --- a/lib/Scanner/ExternalKaspersky.php +++ b/lib/Scanner/ExternalKaspersky.php @@ -42,7 +42,7 @@ public function initScanner() { if (!($avHost && $avPort)) { throw new \RuntimeException('The Kaspersky port and host are not set up.'); } - $this->writeHandle = fopen("php://temp", 'w+'); + $this->writeHandle = fopen('php://temp', 'w+'); } /** @@ -51,7 +51,7 @@ public function initScanner() { protected function writeChunk($chunk) { if (ftell($this->writeHandle) > $this->chunkSize) { $this->scanBuffer(); - $this->writeHandle = fopen("php://temp", 'w+'); + $this->writeHandle = fopen('php://temp', 'w+'); } parent::writeChunk($chunk); } @@ -66,7 +66,7 @@ protected function scanBuffer(): void { $body = base64_encode($body); $response = $this->clientService->newClient()->post("$avHost:$avPort/api/v3.0/scanmemory", [ 'json' => [ - 'timeout' => "60000", + 'timeout' => '60000', 'object' => $body, ], 'connect_timeout' => 5, @@ -85,14 +85,14 @@ protected function scanBuffer(): void { } elseif (substr($scanResult, 0, 11) === 'NON_SCANNED' && $this->status->getNumericStatus() != Status::SCANRESULT_INFECTED) { if ($scanResult === 'NON_SCANNED (PASSWORD PROTECTED)') { // if we can't scan the file at all, there is no use in trying to scan it again later - $this->status->setNumericStatus(Status::SCANRESULT_CLEAN); + $this->status->setNumericStatus(Status::SCANRESULT_UNSCANNABLE); } else { $this->status->setNumericStatus(Status::SCANRESULT_UNCHECKED); } $this->status->setDetails($scanResult); } else { $this->status->setNumericStatus(Status::SCANRESULT_INFECTED); - if (strpos($scanResult, "DETECT ") === 0) { + if (strpos($scanResult, 'DETECT ') === 0) { $scanResult = substr($scanResult, 7); } if (isset($response['detectionName'])) { diff --git a/lib/Scanner/ICAP.php b/lib/Scanner/ICAP.php index 73caf8ff..d89c8e16 100644 --- a/lib/Scanner/ICAP.php +++ b/lib/Scanner/ICAP.php @@ -56,31 +56,31 @@ public function __construct( public function initScanner() { parent::initScanner(); - $this->writeHandle = fopen("php://temp", 'w+'); + $this->writeHandle = fopen('php://temp', 'w+'); $path = '/' . trim($this->path, '/'); if (str_contains($path, '.ocTransferId') && str_ends_with($path, '.part')) { [$path] = explode('.ocTransferId', $path, 2); } $remote = $this->request ? $this->request->getRemoteAddress() : null; - $encodedPath = implode("/", array_map("rawurlencode", explode("/", $path))); + $encodedPath = implode('/', array_map('rawurlencode', explode('/', $path))); if ($this->mode === ICAPClient::MODE_REQ_MOD) { $this->icapRequest = $this->icapClient->reqmod($this->service, [ 'Allow' => 204, - "X-Client-IP" => $remote, + 'X-Client-IP' => $remote, ], [ "PUT $encodedPath HTTP/1.0", - "Host: nextcloud" + 'Host: nextcloud' ]); } else { $this->icapRequest = $this->icapClient->respmod($this->service, [ 'Allow' => 204, - "X-Client-IP" => $remote, + 'X-Client-IP' => $remote, ], [ "GET $encodedPath HTTP/1.0", - "Host: nextcloud", + 'Host: nextcloud', ], [ - "HTTP/1.0 200 OK", - "Content-Length: 1", // a dummy, non-zero, content length seems to be enough + 'HTTP/1.0 200 OK', + 'Content-Length: 1', // a dummy, non-zero, content length seems to be enough ]); } } @@ -96,7 +96,7 @@ private function flushBuffer() { rewind($this->writeHandle); $data = stream_get_contents($this->writeHandle); $this->icapRequest->write($data); - $this->writeHandle = fopen("php://temp", 'w+'); + $this->writeHandle = fopen('php://temp', 'w+'); } protected function scanBuffer() { @@ -105,9 +105,10 @@ protected function scanBuffer() { $code = $response->getStatus()->getCode(); $this->status->setNumericStatus(Status::SCANRESULT_CLEAN); + $icapHeaders = $response->getIcapHeaders(); if ($code === 200 || $code === 204) { // c-icap/clamav reports this header - $virus = $response->getIcapHeaders()[$this->virusHeader] ?? false; + $virus = $icapHeaders[$this->virusHeader] ?? false; if ($virus) { $this->status->setNumericStatus(Status::SCANRESULT_INFECTED); $this->status->setDetails($virus); @@ -120,6 +121,17 @@ protected function scanBuffer() { } } elseif ($code === 202) { $this->status->setNumericStatus(Status::SCANRESULT_UNCHECKED); + } elseif ($code === 500 && isset($icapHeaders['X-Error-Code'])) { + $uncheckableErrors = ['decode_error', 'max_archive_layers_exceeded', 'password_protected']; + $blockedErrors = ['file_type_blocked', 'file_extension_blocked']; + $icapErrorCode = $icapHeaders['X-Error-Code']; + if (in_array($icapErrorCode, $uncheckableErrors)) { + $this->status->setNumericStatus(Status::SCANRESULT_UNSCANNABLE); + } elseif (in_array($icapErrorCode, $blockedErrors)) { + $this->status->setNumericStatus(Status::SCANRESULT_INFECTED); + } else { + throw new \RuntimeException('Invalid response from ICAP server, got error code ' . $icapErrorCode); + } } else { throw new \RuntimeException('Invalid response from ICAP server'); } diff --git a/lib/Scanner/ScannerBase.php b/lib/Scanner/ScannerBase.php index 46c08dbf..fb21b7d8 100644 --- a/lib/Scanner/ScannerBase.php +++ b/lib/Scanner/ScannerBase.php @@ -28,7 +28,7 @@ abstract class ScannerBase implements IScanner { protected int $byteCount; - /** @var resource */ + /** @var resource */ protected $writeHandle; protected AppConfig $appConfig; diff --git a/lib/Status.php b/lib/Status.php index d01beb24..1a7c6b01 100644 --- a/lib/Status.php +++ b/lib/Status.php @@ -12,23 +12,28 @@ use Psr\Log\LoggerInterface; class Status { - /* + /** * The file was not checked (e.g. because the AV daemon wasn't running). */ public const SCANRESULT_UNCHECKED = -1; - /* + /** * The file was checked and found to be clean. */ public const SCANRESULT_CLEAN = 0; - /* + /** * The file was checked and found to be infected. */ public const SCANRESULT_INFECTED = 1; - /* - * Should be SCANRESULT_UNCHECKED | SCANRESULT_INFECTED | SCANRESULT_CLEAN + /** + * The file cannot be checked (e.g. because it is password protected) + */ + public const SCANRESULT_UNSCANNABLE = 2; + + /** + * Should be SCANRESULT_UNCHECKED | SCANRESULT_INFECTED | SCANRESULT_CLEAN | SCANRESULT_UNSCANNABLE */ protected $numericStatus = self::SCANRESULT_UNCHECKED; @@ -37,12 +42,15 @@ class Status { */ protected $details = ''; + private bool $blockUnscannable = false; + protected RuleMapper $ruleMapper; protected LoggerInterface $logger; - public function __construct(RuleMapper $ruleMapper, LoggerInterface $logger) { + public function __construct(RuleMapper $ruleMapper, LoggerInterface $logger, AppConfig $config) { $this->ruleMapper = $ruleMapper; $this->logger = $logger; + $this->blockUnscannable = $config->getAvBlockUnscannable(); } /** @@ -71,18 +79,18 @@ public function setDetails(string $details): void { /** * @param string $rawResponse - * @param integer $result + * @param ?integer $result * * @return void */ - public function parseResponse($rawResponse, $result = null) { + public function parseResponse(string $rawResponse, ?int $result = null) { $matches = []; if (is_null($result)) { // Daemon or socket mode try { $allRules = $this->getResponseRules(); } catch (\Exception $e) { - $this->logger->error(__METHOD__.', exception: '.$e->getMessage(), ['app' => 'files_antivirus']); + $this->logger->error(__METHOD__ . ', exception: ' . $e->getMessage(), ['app' => 'files_antivirus']); return; } @@ -106,7 +114,7 @@ public function parseResponse($rawResponse, $result = null) { // Adding the ASCII text range 32..126 (excluding '`') of the raw socket response to the details. $response = filter_var($rawResponse, FILTER_UNSAFE_RAW, FILTER_FLAG_STRIP_LOW | FILTER_FLAG_STRIP_HIGH | FILTER_FLAG_STRIP_BACKTICK); if (strlen($response) > 512) { - $response = substr($response, 0, 509) . "..."; + $response = substr($response, 0, 509) . '...'; } $this->details = 'No matching rule for response [' . $response . ']. Please check antivirus rules configuration.'; } @@ -132,7 +140,7 @@ public function parseResponse($rawResponse, $result = null) { break; case self::SCANRESULT_UNCHECKED: if (!$this->details) { - $this->details = 'No matching rule for exit code ' . $this->numericStatus .'. Please check antivirus rules configuration.' ; + $this->details = 'No matching rule for exit code ' . $this->numericStatus . '. Please check antivirus rules configuration.' ; } } } @@ -165,6 +173,13 @@ public function dispatch(Item $item): void { case self::SCANRESULT_CLEAN: $item->processClean(); break; + case self::SCANRESULT_UNSCANNABLE: + if ($this->blockUnscannable) { + $item->processInfected($this); + } else { + $item->processClean(); + } + break; } } } diff --git a/lib/StatusFactory.php b/lib/StatusFactory.php index 8363f28a..e3c907a2 100644 --- a/lib/StatusFactory.php +++ b/lib/StatusFactory.php @@ -12,16 +12,23 @@ class StatusFactory { private RuleMapper $ruleMapper; private LoggerInterface $logger; + private AppConfig $config; - public function __construct(RuleMapper $ruleMapper, LoggerInterface $logger) { + public function __construct( + RuleMapper $ruleMapper, + LoggerInterface $logger, + AppConfig $config + ) { $this->ruleMapper = $ruleMapper; $this->logger = $logger; + $this->config = $config; } public function newStatus(): Status { return new Status( $this->ruleMapper, - $this->logger + $this->logger, + $this->config, ); } } diff --git a/templates/settings.php b/templates/settings.php index 7424d5e3..699dce23 100644 --- a/templates/settings.php +++ b/templates/settings.php @@ -121,6 +121,15 @@ + + + + "" + title="t('Block unscannable files (such as encrypted archives)'));?>" + /> + + + diff --git a/tests/AvirWrapperTest.php b/tests/AvirWrapperTest.php index dbfc44f9..86266178 100644 --- a/tests/AvirWrapperTest.php +++ b/tests/AvirWrapperTest.php @@ -76,6 +76,7 @@ protected function setUp(): void { 'eventDispatcher' => $this->createMock(EventDispatcherInterface::class), 'trashEnabled' => true, 'mount_point' => '/' . self::UID . '/files/', + 'block_unscannable' => false, ]); $this->config->expects($this->any()) diff --git a/tests/BackgroundScannerTest.php b/tests/BackgroundScannerTest.php index 773cf351..d9756d24 100644 --- a/tests/BackgroundScannerTest.php +++ b/tests/BackgroundScannerTest.php @@ -39,7 +39,7 @@ class BackgroundScannerTest extends TestBase { protected function setUp(): void { parent::setUp(); - $this->createUser("av", "av"); + $this->createUser('av', 'av'); $storage = new TemporaryHome(); $storage->mkdir('files'); $storage->getScanner()->scan(''); @@ -47,13 +47,13 @@ protected function setUp(): void { $external = new Temporary(); $external->getScanner()->scan(''); - $this->registerMount("av", $storage, "av"); - $this->registerMount("av", $external, "av/files/external"); + $this->registerMount('av', $storage, 'av'); + $this->registerMount('av', $external, 'av/files/external'); - $this->loginAsUser("av"); + $this->loginAsUser('av'); /** @var IRootFolder $root */ $root = \OC::$server->get(IRootFolder::class); - $this->homeDirectory = $root->getUserFolder("av"); + $this->homeDirectory = $root->getUserFolder('av'); $this->externalDirectory = $this->homeDirectory->get('external'); } @@ -87,7 +87,7 @@ private function getBackgroundScanner(): BackgroundScanner { ->disableOriginalConstructor() ->onlyMethods(['getNumericStatus', 'getDetails'])->getMock(); $status->method('getNumericStatus')->willReturn(Status::SCANRESULT_CLEAN); - $status->method('getDetails')->willReturn(""); + $status->method('getDetails')->willReturn(''); $scanner = $this->createMock(IScanner::class); $scanner->method('scan') ->willReturn($status); @@ -123,8 +123,8 @@ public function testGetUnscannedFiles() { $this->markAllScanned(); $scanner = $this->getBackgroundScanner(); - $newFileId = $this->homeDirectory->newFile("foo", "bar")->getId(); - $this->homeDirectory->getParent()->newFile("outside", "bar")->getId(); + $newFileId = $this->homeDirectory->newFile('foo', 'bar')->getId(); + $this->homeDirectory->getParent()->newFile('outside', 'bar')->getId(); $outdated = iterator_to_array($scanner->getUnscannedFiles()); $this->assertEquals([$newFileId], $outdated); @@ -134,14 +134,14 @@ public function testGetUnscannedFilesExternal() { $this->markAllScanned(); $scanner = $this->getBackgroundScanner(); - $newFileId = $this->homeDirectory->newFile("external/foo2", "bar2")->getId(); + $newFileId = $this->homeDirectory->newFile('external/foo2', 'bar2')->getId(); $outdated = iterator_to_array($scanner->getUnscannedFiles()); $this->assertEquals([$newFileId], $outdated); } public function testGetOutdatedFiles() { - $newFileId = $this->homeDirectory->newFile("foo", "bar")->getId(); + $newFileId = $this->homeDirectory->newFile('foo', 'bar')->getId(); $this->markAllScanned(); $scanner = $this->getBackgroundScanner(); @@ -158,7 +158,7 @@ public function testTestScanFiles() { $this->markAllScanned(); $scanner = $this->getBackgroundScanner(); - $newFileId = $this->homeDirectory->newFile("foo", "bar")->getId(); + $newFileId = $this->homeDirectory->newFile('foo', 'bar')->getId(); $outdated = iterator_to_array($scanner->getUnscannedFiles()); $this->assertEquals([$newFileId], $outdated); diff --git a/tests/Db/RuleTest.php b/tests/Db/RuleTest.php index ba4445d2..de520ffd 100644 --- a/tests/Db/RuleTest.php +++ b/tests/Db/RuleTest.php @@ -22,7 +22,7 @@ public function testJsonSerialize() { 'statusType' => Rule::RULE_TYPE_CODE, 'result' => 0, 'match' => '', - 'description' => "", + 'description' => '', 'status' => Status::SCANRESULT_CLEAN ]; $expected = [ @@ -30,7 +30,7 @@ public function testJsonSerialize() { 'status_type' => Rule::RULE_TYPE_CODE, 'result' => 0, 'match' => '', - 'description' => "", + 'description' => '', 'status' => Status::SCANRESULT_CLEAN ]; diff --git a/tests/DummyClam.php b/tests/DummyClam.php index b907278e..aeaf4b1e 100644 --- a/tests/DummyClam.php +++ b/tests/DummyClam.php @@ -74,7 +74,7 @@ protected function handleConnection($connection) { if (!$isAborted) { $response = strpos($buffer, self::TEST_SIGNATURE) !== false - ? "Ohoho: Criminal.Joboholic FOUND" + ? 'Ohoho: Criminal.Joboholic FOUND' : 'Scanned OK' ; //echo str_replace('0', '', $buffer) . $response; diff --git a/tests/ICAP/ICAPClientTest.php b/tests/ICAP/ICAPClientTest.php index a29e2ac2..1f7be0c6 100644 --- a/tests/ICAP/ICAPClientTest.php +++ b/tests/ICAP/ICAPClientTest.php @@ -15,7 +15,7 @@ class ICAPClientTest extends TestCase { public function testConnect_ShouldThrowRuntimeException() { $this->expectException(\RuntimeException::class); $this->expectExceptionMessageMatches('/Cannot connect to "tcp\:\/\/nothinghere\:8080"\: .*/'); - $icapClient = new ICAPClient("nothinghere", 8080, 2); - $icapClient->respmod("myservice", [], [], []); + $icapClient = new ICAPClient('nothinghere', 8080, 2); + $icapClient->respmod('myservice', [], [], []); } } diff --git a/tests/Scanner/ExternalKasperskyTest.php b/tests/Scanner/ExternalKasperskyTest.php index b5f4a7cc..9018b0af 100644 --- a/tests/Scanner/ExternalKasperskyTest.php +++ b/tests/Scanner/ExternalKasperskyTest.php @@ -21,7 +21,7 @@ class ExternalKasperskyTest extends ScannerBaseTest { protected function getScanner(): ScannerBase { if (!getenv('KASPERSKY_HOST') || !getenv('KASPERSKY_PORT')) { - $this->markTestSkipped("Set KASPERSKY_HOST and KASPERSKY_PORT to enable kaspersky tests"); + $this->markTestSkipped('Set KASPERSKY_HOST and KASPERSKY_PORT to enable kaspersky tests'); } $logger = $this->createMock(LoggerInterface::class); diff --git a/tests/Scanner/ICAPTest.php b/tests/Scanner/ICAPTest.php index 48cd89eb..de3dd767 100644 --- a/tests/Scanner/ICAPTest.php +++ b/tests/Scanner/ICAPTest.php @@ -20,7 +20,7 @@ class ICAPTest extends ScannerBaseTest { protected function getScanner(): ScannerBase { if (!getenv('ICAP_HOST') || !getenv('ICAP_PORT') || !getenv('ICAP_REQUEST') || !getenv('ICAP_HEADER') || !getenv('ICAP_MODE')) { - $this->markTestSkipped("Set ICAP_HOST, ICAP_PORT, ICAP_REQUEST, ICAP_MODE and ICAP_HEADER to enable icap tests"); + $this->markTestSkipped('Set ICAP_HOST, ICAP_PORT, ICAP_REQUEST, ICAP_MODE and ICAP_HEADER to enable icap tests'); } $logger = $this->createMock(LoggerInterface::class); diff --git a/tests/Scanner/ScannerBaseTest.php b/tests/Scanner/ScannerBaseTest.php index 76a42681..53ed838b 100644 --- a/tests/Scanner/ScannerBaseTest.php +++ b/tests/Scanner/ScannerBaseTest.php @@ -17,7 +17,7 @@ abstract protected function getScanner(): ScannerBase; public function testScanClean() { $scanner = $this->getScanner(); - $status = $scanner->scanString("foo"); + $status = $scanner->scanString('foo'); $this->assertEquals($status->getNumericStatus(), Status::SCANRESULT_CLEAN); } public function testScanEicar() { diff --git a/tests/StatusTest.php b/tests/StatusTest.php index 47262a7e..1e460d7d 100644 --- a/tests/StatusTest.php +++ b/tests/StatusTest.php @@ -15,46 +15,51 @@ * @group DB */ class StatusTest extends TestBase { - + // See OCA\Files_Antivirus\Status::init for details public const TEST_CLEAN = 0; public const TEST_INFECTED = 1; public const TEST_ERROR = 40; - - protected $ruleMapper; + protected RuleMapper $ruleMapper; + private bool $blockUnscannable = false; protected function setUp(): void { parent::setUp(); $this->ruleMapper = new RuleMapper($this->db); $this->ruleMapper->deleteAll(); $this->ruleMapper->populate(); + $this->config->method('getAvBlockUnscannable') + ->willReturnCallback(function () { + return $this->blockUnscannable; + }); } - + public function testParseResponse() { // Testing status codes $testStatus = new \OCA\Files_Antivirus\Status( $this->ruleMapper, - $this->createMock(LoggerInterface::class) + $this->createMock(LoggerInterface::class), + $this->config, ); - + $testStatus->parseResponse('dummy : OK', self::TEST_CLEAN); $cleanScan = $testStatus->getNumericStatus(); $this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_CLEAN, $cleanScan); - $this->assertEquals("", $testStatus->getDetails()); - - $scanOutput = "Thu Oct 28 13:02:19 2010 -> /tmp/kitten: Heuristics.Broken.Executable FOUND "; + $this->assertEquals('', $testStatus->getDetails()); + + $scanOutput = 'Thu Oct 28 13:02:19 2010 -> /tmp/kitten: Heuristics.Broken.Executable FOUND '; $testStatus->parseResponse($scanOutput, self::TEST_INFECTED); $infectedScan = $testStatus->getNumericStatus(); $this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_INFECTED, $infectedScan); $this->assertEquals('Heuristics.Broken.Executable', $testStatus->getDetails()); - + $testStatus->parseResponse('dummy', self::TEST_ERROR); $failedScan = $testStatus->getNumericStatus(); $this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_UNCHECKED, $failedScan); $this->assertEquals('Unknown option passed.', $testStatus->getDetails()); - - + + // Testing raw output (e.g. daemon mode) $assertDetailsWithResponse = function ($response) use ($testStatus) { $expected = "No matching rule for response [$response]. Please check antivirus rules configuration."; @@ -66,32 +71,32 @@ public function testParseResponse() { $failedScan2 = $testStatus->getNumericStatus(); $this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_UNCHECKED, $failedScan2); $assertDetailsWithResponse(''); - + // No rules matched result is unknown too $testStatus->parseResponse('123dc'); $failedScan3 = $testStatus->getNumericStatus(); $this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_UNCHECKED, $failedScan3); $assertDetailsWithResponse('123dc'); - + // Raw result is added to details when no rule matched (only ASCII text range 32..126 excluding '`'). for ($c = 0; $c < 256; $c++) { $testStatus->parseResponse(chr($c)); $expected = $c < 32 || $c > 126 || chr($c) == '`' ? '' : chr($c); $assertDetailsWithResponse($expected); } - + // Raw result in details is truncated at 512 chars. $testStatus->parseResponse(str_repeat('a', 512)); $assertDetailsWithResponse(str_repeat('a', 512)); $testStatus->parseResponse(str_repeat('a', 513)); $assertDetailsWithResponse(str_repeat('a', 509) . '...'); - + // File is clean $testStatus->parseResponse('Thu Oct 28 13:02:19 2010 -> /tmp/kitten : OK'); $cleanScan2 = $testStatus->getNumericStatus(); $this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_CLEAN, $cleanScan2); $this->assertEquals('', $testStatus->getDetails()); - + // File is infected $testStatus->parseResponse('Thu Oct 28 13:02:19 2010 -> /tmp/kitten: Heuristics.Broken.Kitten FOUND'); $infectedScan2 = $testStatus->getNumericStatus(); diff --git a/tests/TemporaryHome.php b/tests/TemporaryHome.php index 09c20f81..1af2b9e0 100644 --- a/tests/TemporaryHome.php +++ b/tests/TemporaryHome.php @@ -18,7 +18,7 @@ public function __construct($arguments = null) { } public function getId() { - return "home::" . $this->id; + return 'home::' . $this->id; } } diff --git a/tests/TestBase.php b/tests/TestBase.php index 4d22b78c..b5a008ae 100644 --- a/tests/TestBase.php +++ b/tests/TestBase.php @@ -39,7 +39,7 @@ protected function setUp(): void { $this->config = $this->getMockBuilder(AppConfig::class) ->disableOriginalConstructor() - ->setMethods(['getAvPath', 'getAvChunkSize', 'getAvMode', 'getAppValue', 'getAvHost', 'getAvPort']) + ->setMethods(['getAvPath', 'getAvChunkSize', 'getAvMode', 'getAppValue', 'getAvHost', 'getAvPort', 'getAvBlockUnscannable']) ->getMock(); $this->config->expects($this->any()) @@ -62,8 +62,8 @@ protected function setUp(): void { ->will($this->returnValue('5555')); $this->l10n = $this->getMockBuilder(IL10N::class) - ->disableOriginalConstructor() - ->getMock(); + ->disableOriginalConstructor() + ->getMock(); $this->l10n->method('t')->will($this->returnArgument(0)); } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 0499cc64..09e4c540 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -13,7 +13,7 @@ define('PHPUNIT_RUN', 1); } -require_once __DIR__.'/../../../lib/base.php'; +require_once __DIR__ . '/../../../lib/base.php'; if (!class_exists('PHPUnit_Framework_TestCase') && !class_exists('\PHPUnit\Framework\TestCase')) { require_once('PHPUnit/Autoload.php');