Merge pull request #330 from nextcloud/bugfix/noid/fix-negative-file-…

…name-match

fix(filename): Exclude part/transfer files from checks

lib/Operation.php

@@ -36,6 +36,7 @@
 use OCP\WorkflowEngine\IManager;
 use OCP\WorkflowEngine\IRuleMatcher;
 use OCP\WorkflowEngine\ISpecificOperation;
+use Psr\Log\LoggerInterface;
 use ReflectionClass;
 use UnexpectedValueException;
 
@@ -47,14 +48,24 @@ class Operation implements IComplexOperation, ISpecificOperation {
 	private File $fileEntity;
 	private IMountManager $mountManager;
 	private IRootFolder $rootFolder;
-
-	public function __construct(IManager $manager, IL10N $l, IURLGenerator $urlGenerator, File $fileEntity, IMountManager $mountManager, IRootFolder $rootFolder) {
+	protected LoggerInterface $logger;
+
+	public function __construct(
+		IManager $manager,
+		IL10N $l,
+		IURLGenerator $urlGenerator,
+		File $fileEntity,
+		IMountManager $mountManager,
+		IRootFolder $rootFolder,
+		LoggerInterface $logger
+	) {
 		$this->manager = $manager;
 		$this->l = $l;
 		$this->urlGenerator = $urlGenerator;
 		$this->fileEntity = $fileEntity;
 		$this->mountManager = $mountManager;
 		$this->rootFolder = $rootFolder;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -83,6 +94,8 @@ public function checkFileAccess(IStorage $storage, string $path, bool $isDir = f
 		$this->nestingLevel--;
 
 		if (!empty($match)) {
+			$e = new \RuntimeException('Access denied for path ' . $path . ' that is ' . ($isDir ? '' : 'not ') . 'a directory and matches rules: ' . json_encode($match));
+			$this->logger->debug($e->getMessage(), ['exception' => $e]);
 			// All Checks of one operation matched: prevent access
 			throw new ForbiddenException('Access denied', false);
 		}
@@ -111,6 +124,10 @@ protected function isBlockablePath(IStorage $storage, string $path): bool {
 			return false;
 		}
 
+		if (preg_match('/\.ocTransferId\d+\.part$/i', $path)) {
+			return false;
+		}
+
 		// '', admin, 'files', 'path/to/file.txt'
 		$segment = explode('/', $fullPath, 4);
 

tests/Integration/features/bootstrap/FeatureContext.php

@@ -82,6 +82,14 @@ public function cleanUpBetweenTests() {
 			$this->userDeletesFile('test1', 'file', '/foobar.txt');
 		} catch (\Exception $e) {
 		}
+		try {
+			$this->userDeletesFile('test1', 'file', '/definitely.notexe');
+		} catch (\Exception $e) {
+		}
+		try {
+			$this->emptyTrashbin('test1');
+		} catch (\Exception $e) {
+		}
 	}
 
 	/**

tests/Integration/features/bootstrap/WebDav.php

@@ -655,6 +655,16 @@ public function userDeletesFile($user, $type, $file) {
 		}
 	}
 
+	/**
+	 * @When User "([^"]*)" empties trashbin
+	 * @param string $user user
+	 */
+	public function emptyTrashbin($user) {
+		$client = $this->getSabreClient($user);
+		$response = $client->request('DELETE', $this->makeSabrePath($user, 'trash', 'trashbin'));
+		Assert::assertEquals(204, $response['statusCode']);
+	}
+
 	/**
 	 * @Given User :user created a folder :destination
 	 * @param string $user
@@ -960,7 +970,12 @@ public function userSeesNoFilesInTheTrashbin($user) {
 		// Remove the root entry to only have the directory listing
 		unset($response['/remote.php/dav/trashbin/' . $user . '/trash/']);
 
-		Assert::assertEquals(0, count($response));
+		$count = count($response);
+		if ($count !== 0) {
+			var_dump($response);
+		}
+
+		Assert::assertEquals(0, $count);
 	}
 
 

tests/Integration/features/filename.feature

@@ -0,0 +1,21 @@
+Feature: Author
+  Background:
+    Given user "test1" exists
+    Given as user "test1"
+    And using new dav path
+
+  Scenario: Creating file with negative name match
+    Given user "admin" creates global flow with 200
+    | name      | Admin flow                       |
+    | class     | OCA\FilesAccessControl\Operation |
+    | entity    | OCA\WorkflowEngine\Entity\File   |
+    | events    | []                               |
+    | operation | deny                             |
+    | checks-0  | {"class":"OCA\\WorkflowEngine\\Check\\FileName", "operator": "!matches", "value": "\/\\.notexe$\/i"} |
+    | checks-1  | {"class":"OCA\\WorkflowEngine\\Check\\FileMimeType", "operator": "matches", "value": "\/^application\\\/(octet-stream\|x-ms-dos-executable\|x-msi\|x-msdos-program)$\/i"} |
+    And User "test1" uploads file "data/dosexecutable.exe" to "/definitely.notexe"
+    Then The webdav response should have a status code "201"
+    And User "test1" uploads file "data/dosexecutable.exe" to "/definitely.exe"
+    Then The webdav response should have a status code "403"
+    Then User "test1" sees no files in the trashbin
+