Merge pull request #378 from nextcloud/unscannable

[wip] feat: add option whether to block unscannable files

.github/workflows/package.yml

@@ -10,18 +10,18 @@ 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
           chmod +x krankerl
       - 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

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
 	 *

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

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) {

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);

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 <info>$count</info> files");
 		if ($count === $max) {
-			$output->writeln("  there might still be unscanned files remaining");
+			$output->writeln('  there might still be unscanned files remaining');
 		}
 
 		return 0;

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 <info>scanned</info> or <info>unscanned</info>");
+			->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 <info>scanned</info> or <info>unscanned</info>');
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output): int {

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 <info>clean</info>";
+				$status = 'is <info>clean</info>';
 				$exit = 0;
 				break;
 			case \OCA\Files_Antivirus\Status::SCANRESULT_INFECTED:
-				$status = "is <error>infected</error>";
+				$status = 'is <error>infected</error>';
 				$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("<info>$path</info> $status$details");
 

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;

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("<error>❌ $details</error>");
 			return 1;
 		} elseif ($result->getNumericStatus() === Status::SCANRESULT_UNCHECKED) {
-			$output->writeln("<comment>- file not scanned or scan still pending</comment>");
+			$output->writeln('<comment>- file not scanned or scan still pending</comment>');
 		} else {
-			$output->writeln("<info>✓</info>");
+			$output->writeln('<info>✓</info>');
 		}
 
-		$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("<error>❌ file not detected $details</error>");
 			return 1;
 		} elseif ($result->getNumericStatus() === Status::SCANRESULT_UNCHECKED) {
-			$output->writeln("<comment>- file not scanned or scan still pending</comment>");
+			$output->writeln('<comment>- file not scanned or scan still pending</comment>');
+		} elseif ($result->getNumericStatus() === Status::SCANRESULT_UNSCANNABLE) {
+			$output->writeln('<comment>- file could not be scanned</comment>');
 		} else {
-			$output->writeln("<info>✓</info>");
+			$output->writeln('<info>✓</info>');
 		}
 
 		// 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("<error>❌ file not detected $details</error>");
 			return 1;
 		} elseif ($result->getNumericStatus() === Status::SCANRESULT_UNCHECKED) {
-			$output->writeln("<comment>- file not scanned or scan still pending</comment>");
+			$output->writeln('<comment>- file not scanned or scan still pending</comment>');
+		} elseif ($result->getNumericStatus() === Status::SCANRESULT_UNSCANNABLE) {
+			$output->writeln('<comment>- file could not be scanned</comment>');
 		} else {
-			$output->writeln("<info>✓</info>");
+			$output->writeln('<info>✓</info>');
 		}
 
 		return 0;

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) {

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;
 

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);

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);

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);
 		}