From fb709bc21f7b226424df7110019447e1870fb24a Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Wed, 26 Aug 2026 20:27:59 +0200 Subject: [PATCH] fix: Don't apply the ACL wrapper to trash storage on separate storages getBaseStorageForFolderRootJail() skips the ACL wrapper for the trash storage because the trash backend does its own ACL filtering, but the separate storage path never got the same guard. createFolder() always uses separate storages now, so every recently created folder ends up with the wrapper on its trash. The visible effect is a permanent delete that can never succeed. ACLStorageWrapper sits below the jail, so it sees the trash paths, and its unlink() runs canDeleteTree() over them. A deny delete rule follows a file's id into the trash, so permanently deleting the folder that file was in fails with "Failed to remove item from trashbin", even though the trash backend already decided the user is allowed to delete that item. Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- lib/Mount/FolderStorageManager.php | 3 ++- tests/Trash/TrashBackendTest.php | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/Mount/FolderStorageManager.php b/lib/Mount/FolderStorageManager.php index f2b084911..8cd0f9c63 100644 --- a/lib/Mount/FolderStorageManager.php +++ b/lib/Mount/FolderStorageManager.php @@ -109,7 +109,8 @@ private function getBaseStorageForFolderSeparate( $storage = $this->getBaseStorageForFolderSeparateStorageLocal($folderId, $init); } - if ($folder?->acl && $user) { + // apply acl before jail, trash doesn't get the ACL wrapper as it does its own ACL filtering + if ($folder?->acl && $user && $type !== 'trash') { $aclManager = $this->aclManagerFactory->getACLManager($user); $storage = new ACLStorageWrapper([ 'storage' => $storage, diff --git a/tests/Trash/TrashBackendTest.php b/tests/Trash/TrashBackendTest.php index 4ab316b7b..384fb684d 100644 --- a/tests/Trash/TrashBackendTest.php +++ b/tests/Trash/TrashBackendTest.php @@ -226,6 +226,30 @@ public function testHideDeletedTrashItemInDeletedParentFolderAcl(): void { $this->logout(); } + public function testRemoveTrashedFolderWithRestrictedChild(): void { + $this->loginAsUser('manager'); + + $folder = $this->managerUserFolder->newFolder("{$this->folderName}/folder"); + $child = $folder->newFile('file.txt', 'content'); + + $this->trashBackend->moveToTrash($folder->getStorage(), $folder->getInternalPath()); + $this->assertFalse($this->managerUserFolder->nodeExists("{$this->folderName}/folder")); + + // the rule follows the child's file id into the trash, where the storage must not + // enforce it: the trash backend does its own ACL filtering + $this->ruleManager->saveRule(new Rule(new UserMapping('user', 'manager'), $child->getId(), Constants::PERMISSION_DELETE, 0)); + + $items = $this->trashBackend->listTrashRoot($this->managerUser); + $this->assertCount(1, $items); + + $this->trashBackend->removeItem($items[0]); + + $this->assertCount(0, $this->trashBackend->listTrashRoot($this->managerUser)); + $this->assertCount(0, $this->trashManager->listTrashForFolders([$this->folderId])); + + $this->logout(); + } + public function testWrongOriginalLocation(): void { $shareManager = Server::get(Share\IManager::class);