Merge pull request #33 from GDATASoftwareAG/behaviour-improvements

Behaviour improvements
GDATASoftwareAG · Jun 5, 2024 · e2424ee · e2424ee
2 parents 2056dad + 6ecc6d0
commit e2424ee
Show file tree

Hide file tree

Showing 12 changed files with 54 additions and 36 deletions.
diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
@@ -12,7 +12,16 @@
 	// "customizations": {},
 
 	// Use 'forwardPorts' to make a list of ports inside the container available locally.
-	"forwardPorts": [8080]
+	"forwardPorts": [
+		8080
+	],
+	"customizations": {
+		"vscode": {
+			"extensions": [
+				"CharlieGerard.pride-vscode-themes"
+			]
+		}
+	}
 
 	// Use 'postCreateCommand' to run commands after the container is created.
 	// "postCreateCommand": "sudo chmod a+x \"$(pwd)\" && sudo rm -rf /var/www/html && sudo ln -s \"$(pwd)\" /var/www/html"

diff --git a/.gitignore b/.gitignore
@@ -1,6 +1,8 @@
 # SPDX-FileCopyrightText: Lennart Dohmann <[email protected]>
 # SPDX-License-Identifier: AGPL-3.0-or-later
+.env-local
 .idea
+*.local
 *.iml
 /build/
 node_modules/
@@ -59,4 +61,4 @@ move_app.sh
 dev-environment*/
 js/
 *.cache
-.uuid
+.uuid
diff --git a/composer.json b/composer.json
@@ -9,7 +9,8 @@
 		}
 	],
 	"require": {
-		"gdata/vaas": "^8.0.2"
+		"gdata/vaas": "^8.0.2",
+		"coduo/php-humanizer": "^5.0"
 	},
 	"require-dev": {
 		"nextcloud/ocp": "dev-stable28",

diff --git a/install.sh b/install.sh
@@ -38,5 +38,5 @@ docker exec --user www-data -it nextcloud-container php occ config:app:set gdata
 docker exec --user www-data -it nextcloud-container php occ log:manage --level DEBUG
 docker exec --user www-data -it nextcloud-container php occ app:disable firstrunwizard
 
-
+source *.local
 # docker exec --user www-data -it nextcloud-container php cron.php
diff --git a/lib/Activity/Provider.php b/lib/Activity/Provider.php
@@ -31,8 +31,7 @@
 use OCP\L10N\IFactory;
 use Psr\Log\LoggerInterface;
 
-class Provider implements IProvider
-{
+class Provider implements IProvider {
 	public const TYPE_VIRUS_DETECTED = 'virus_detected';
 
 	public const SUBJECT_VIRUS_DETECTED = 'virus_detected';
@@ -48,15 +47,13 @@ class Provider implements IProvider
 	private $urlGenerator;
 	private LoggerInterface $logger;
 
-	public function __construct(IFactory $languageFactory, IURLGenerator $urlGenerator, LoggerInterface $logger)
-	{
+	public function __construct(IFactory $languageFactory, IURLGenerator $urlGenerator, LoggerInterface $logger) {
 		$this->languageFactory = $languageFactory;
 		$this->urlGenerator = $urlGenerator;
 		$this->logger = $logger;
 	}
 
-	public function parse($language, IEvent $event, ?IEvent $previousEvent = null)
-	{
+	public function parse($language, IEvent $event, ?IEvent $previousEvent = null) {
 		if ($event->getApp() !== Application::APP_ID || $event->getType() !== self::TYPE_VIRUS_DETECTED) {
 			throw new \InvalidArgumentException();
 		}
@@ -127,8 +124,7 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null)
 		return $event;
 	}
 
-	private function setSubjects(IEvent $event, string $subject, array $parameters): void
-	{
+	private function setSubjects(IEvent $event, string $subject, array $parameters): void {
 		$placeholders = $replacements = [];
 		foreach ($parameters as $placeholder => $parameter) {
 			$placeholders[] = '{' . $placeholder . '}';

diff --git a/lib/AvirWrapper.php b/lib/AvirWrapper.php
@@ -112,7 +112,16 @@ function () use ($path, $logger) {
 					$filesize = $this->filesize($path);
 					$logger->debug("Closing " . $localPath . " with size " . $filesize);
 
-					$verdict = $this->verdictService->scan($localPath);
+					if ($filesize > VerdictService::MAX_FILE_SIZE) {
+						return;
+					}
+
+					try {
+						$verdict = $this->verdictService->scan($localPath);
+					} catch (\Exception $e) {
+						$this->logger->error($e->getMessage(), ['exception' => $e]);
+						return;
+					}
 					$logger->debug("Verdict for  " . $localPath . " is " . $verdict->Verdict->value);
 
 					if ($verdict->Verdict == Verdict::MALICIOUS) {

diff --git a/lib/BackgroundJobs/ScanJob.php b/lib/BackgroundJobs/ScanJob.php
@@ -42,7 +42,6 @@ protected function run($argument): void {
 			return;
 		}
 		$unscannedTagIsDisabled = $this->appConfig->getAppValue(self::APP_ID, 'disableUnscannedTag');
-		$autoScanOnlyNewFiles = $this->appConfig->getAppValue(self::APP_ID, 'scanOnlyNewFiles');
 		$quantity = $this->appConfig->getAppValue(self::APP_ID, 'scanQueueLength');
 		try {
 			$quantity = intval($quantity);
@@ -54,20 +53,13 @@ protected function run($argument): void {
 		$pupTag = $this->tagService->getTag(TagService::PUP);
 		$cleanTag = $this->tagService->getTag(TagService::CLEAN);
 		$unscannedTag = $this->tagService->getTag(TagService::UNSCANNED);
+		$wontScanTag = $this->tagService->getTag(TagService::WONT_SCAN);
 
 		if ($unscannedTagIsDisabled) {
-			if ($autoScanOnlyNewFiles) {
-				$excludedTagIds = [$unscannedTag->getId(), $maliciousTag->getId(), $cleanTag->getId(), $pupTag->getId()];
-			} else {
-				$excludedTagIds = [$unscannedTag->getId()];
-			}
+			$excludedTagIds = [$unscannedTag->getId(), $maliciousTag->getId(), $cleanTag->getId(), $pupTag->getId(), $wontScanTag->getId()];
 			$fileIds = $this->tagService->getFileIdsWithoutTags($excludedTagIds, $quantity);
 		} else {
-			if ($autoScanOnlyNewFiles) {
-				$fileIds = $this->tagService->getFileIdsWithTag(TagService::UNSCANNED, $quantity, 0);
-			} else {
-				$fileIds = $this->tagService->getRandomTaggedFileIds([$maliciousTag->getId(), $cleanTag->getId(), $unscannedTag->getId(), $pupTag->getId()], $quantity, $unscannedTag);
-			}
+			$fileIds = $this->tagService->getFileIdsWithTag(TagService::UNSCANNED, $quantity, 0);
 		}
 
 		$this->logger->debug("Scanning files");

diff --git a/lib/BackgroundJobs/TagUnscannedJob.php b/lib/BackgroundJobs/TagUnscannedJob.php
@@ -46,13 +46,14 @@ protected function run($argument): void {
 		$maliciousTag = $this->tagService->getTag(TagService::MALICIOUS);
 		$pupTag = $this->tagService->getTag(TagService::PUP);
 		$cleanTag = $this->tagService->getTag(TagService::CLEAN);
+		$wontScanTag = $this->tagService->getTag(TagService::WONT_SCAN);
 
-		$excludedTagIds = [$unscannedTag->getId(), $maliciousTag->getId(), $cleanTag->getId(), $pupTag->getId()];
+		$excludedTagIds = [$unscannedTag->getId(), $maliciousTag->getId(), $cleanTag->getId(), $pupTag->getId(), $wontScanTag->getId()];
 
 		$fileIds = $this->tagService->getFileIdsWithoutTags($excludedTagIds, 10000);
 
 		foreach ($fileIds as $fileId) {
-			if ($this->tagService->hasCleanMaliciousOrPupTag($fileId)) {
+			if ($this->tagService->hasAnyButUnscannedTag($fileId)) {
 				continue;
 			}
 			$this->tagService->setTag($fileId, TagService::UNSCANNED);

diff --git a/lib/Controller/ScanController.php b/lib/Controller/ScanController.php
@@ -2,6 +2,7 @@
 
 namespace OCA\GDataVaas\Controller;
 
+use Coduo\PHPHumanizer\NumberHumanizer;
 use OCA\GDataVaas\Service\VerdictService;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\JSONResponse;
@@ -35,7 +36,7 @@ public function scan(int $fileId): JSONResponse {
 			$verdict = $this->verdictService->scanFileById($fileId);
 			return new JSONResponse(['verdict' => $verdict->Verdict->value], 200);
 		} catch (EntityTooLargeException) {
-			return new JSONResponse(['error' => 'File is too large'], 413);
+			return new JSONResponse(['error' => 'File is larger than ' . NumberHumanizer::binarySuffix(VerdictService::MAX_FILE_SIZE, 'de')], 413);
 		} catch (FileDoesNotExistException) {
 			return new JSONResponse(['error' => 'File does not exist'], 404);
 		} catch (InvalidPathException) {

diff --git a/lib/Db/DbFileMapper.php b/lib/Db/DbFileMapper.php
@@ -2,7 +2,6 @@
 
 namespace OCA\GDataVaas\Db;
 
-use OCA\GDataVaas\Service\VerdictService;
 use OCP\AppFramework\Db\QBMapper;
 use OCP\DB\Exception;
 use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -31,7 +30,6 @@ public function getFileIdsWithoutTags(array $excludedTagIds, int $limit): array
 			->where($qb->expr()->notIn('o.systemtagid', $qb->createNamedParameter($excludedTagIds, IQueryBuilder::PARAM_INT_ARRAY)))
 			->orWhere($qb->expr()->isNull('o.systemtagid'))
 			->andWhere($qb->expr()->notLike('m.mimetype', $qb->createNamedParameter('%unix-directory%')))
-			->andWhere($qb->expr()->lte('f.size', $qb->createNamedParameter(VerdictService::MAX_FILE_SIZE)))
 			->andWhere($qb->expr()->like('f.path', $qb->createNamedParameter('files/%')))
 			->orderBy('f.fileid', 'DESC')
 			->setMaxResults($limit);
@@ -61,7 +59,6 @@ public function getFileIdsWithTags(array $includedTagIds, int $limit): array {
 			->leftJoin('f', 'mimetypes', 'm', $qb->expr()->eq('f.mimetype', 'm.id'))
 			->where($qb->expr()->in('o.systemtagid', $qb->createNamedParameter($includedTagIds, IQueryBuilder::PARAM_INT_ARRAY)))
 			->andWhere($qb->expr()->notLike('m.mimetype', $qb->createNamedParameter('%unix-directory%')))
-			->andWhere($qb->expr()->lte('f.size', $qb->createNamedParameter(VerdictService::MAX_FILE_SIZE)))
 			->andWhere($qb->expr()->like('f.path', $qb->createNamedParameter('files/%')))
 			->orderBy('f.fileid', 'DESC')
 			->setMaxResults($limit);

diff --git a/lib/Service/TagService.php b/lib/Service/TagService.php
@@ -16,6 +16,7 @@ class TagService {
 	public const MALICIOUS = 'Malicious';
 	public const PUP = 'Pup';
 	public const UNSCANNED = 'Unscanned';
+	public const WONT_SCAN = 'Won\'t scan';
 
 	private ISystemTagManager $tagService;
 	private ISystemTagObjectMapper $tagMapper;
@@ -77,16 +78,25 @@ public function removeTagFromFile(string $tagName, int $fileId): bool {
 		}
 	}
 
+	public function removeAllTagsFromFile(int $fileId): void {
+		$this->removeTagFromFile(TagService::CLEAN, $fileId);
+		$this->removeTagFromFile(TagService::MALICIOUS, $fileId);
+		$this->removeTagFromFile(TagService::PUP, $fileId);
+		$this->removeTagFromFile(TagService::UNSCANNED, $fileId);
+		$this->removeTagFromFile(TagService::WONT_SCAN, $fileId);
+	}
+
 	/**
 	 * Checks if a file has either CLEAN or MALICIOUS tag and creates these.
 	 * @param int $fileId
 	 * @return bool
 	 */
-	public function hasCleanMaliciousOrPupTag(int $fileId): bool {
+	public function hasAnyButUnscannedTag(int $fileId): bool {
 		if (
 			$this->tagMapper->haveTag([$fileId], 'files', $this->getTag(self::CLEAN)->getId()) ||
 			$this->tagMapper->haveTag([$fileId], 'files', $this->getTag(self::MALICIOUS)->getId()) ||
-			$this->tagMapper->haveTag([$fileId], 'files', $this->getTag(self::PUP)->getId())
+			$this->tagMapper->haveTag([$fileId], 'files', $this->getTag(self::PUP)->getId()) ||
+			$this->tagMapper->haveTag([$fileId], 'files', $this->getTag(self::WONT_SCAN)->getId())
 		) {
 			return true;
 		}
@@ -174,6 +184,7 @@ public function resetAllTags(): void {
 		$this->removeTag(self::MALICIOUS);
 		$this->removeTag(self::UNSCANNED);
 		$this->removeTag(self::PUP);
+		$this->removeTag(self::WONT_SCAN);
 		$this->logger->info("All tags removed");
 	}
 }
diff --git a/lib/Service/VerdictService.php b/lib/Service/VerdictService.php
@@ -71,6 +71,8 @@ public function scanFileById(int $fileId): VaasVerdict {
 		$node = $this->fileService->getNodeFromFileId($fileId);
 		$filePath = $node->getStorage()->getLocalFile($node->getInternalPath());
 		if ($node->getSize() > self::MAX_FILE_SIZE) {
+			$this->tagService->removeAllTagsFromFile($fileId);
+			$this->tagService->setTag($fileId, TagService::WONT_SCAN);
 			throw new EntityTooLargeException("File is too large");
 		}
 
@@ -98,10 +100,7 @@ public function scanFileById(int $fileId): VaasVerdict {
 			. $verdict->Verdict->value . ", Detection: " . $verdict->Detection . ", SHA256: " . $verdict->Sha256 .
 			", FileType: " . $verdict->FileType . ", MimeType: " . $verdict->MimeType . ", UUID: " . $verdict->Guid);
 
-		$this->tagService->removeTagFromFile(TagService::CLEAN, $fileId);
-		$this->tagService->removeTagFromFile(TagService::MALICIOUS, $fileId);
-		$this->tagService->removeTagFromFile(TagService::PUP, $fileId);
-		$this->tagService->removeTagFromFile(TagService::UNSCANNED, $fileId);
+		$this->tagService->removeAllTagsFromFile($fileId);
 
 		switch ($verdict->Verdict->value) {
 			case TagService::CLEAN: