From e6ac77ffde69b139324d696e9b6d96c2f0a4accf Mon Sep 17 00:00:00 2001
From: Sebastian Almberg <83243306+Sebbeben@users.noreply.github.com>
Date: Fri, 20 Feb 2026 23:51:23 +0100
Subject: [PATCH] Add tests for backup/update manager improvements

- Controller tests: auth, CSRF validation, 404 for missing backups, restore disabled check
- UpdateExecutor: deleteLog validation, non-existent file, successful deletion
- BackupManager: deleteBackup validation for missing/non-zip files
---
 .../UpdateManagerControllerTest.php           | 138 ++++++++++++++++++
 tests/Services/System/BackupManagerTest.php   |  10 ++
 tests/Services/System/UpdateExecutorTest.php  |  32 ++++
 3 files changed, 180 insertions(+)
 create mode 100644 tests/Controller/UpdateManagerControllerTest.php

diff --git a/tests/Controller/UpdateManagerControllerTest.php b/tests/Controller/UpdateManagerControllerTest.php
new file mode 100644
index 00000000..b0622918
--- /dev/null
+++ b/tests/Controller/UpdateManagerControllerTest.php
@@ -0,0 +1,138 @@
+<?php
+/*
+ * This file is part of Part-DB (https://github.com/Part-DB/Part-DB-symfony).
+ *
+ *  Copyright (C) 2019 - 2024 Jan Böhmer (https://github.com/jbtronics)
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU Affero General Public License as published
+ *  by the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU Affero General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Affero General Public License
+ *  along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+declare(strict_types=1);
+
+namespace App\Tests\Controller;
+
+use App\Entity\UserSystem\User;
+use PHPUnit\Framework\Attributes\Group;
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+
+#[Group("slow")]
+#[Group("DB")]
+final class UpdateManagerControllerTest extends WebTestCase
+{
+    private function loginAsAdmin($client): void
+    {
+        $entityManager = $client->getContainer()->get('doctrine')->getManager();
+        $userRepository = $entityManager->getRepository(User::class);
+        $user = $userRepository->findOneBy(['name' => 'admin']);
+
+        if (!$user) {
+            $this->markTestSkipped('Admin user not found');
+        }
+
+        $client->loginUser($user);
+    }
+
+    public function testIndexPageRequiresAuth(): void
+    {
+        $client = static::createClient();
+
+        $client->request('GET', '/system/update-manager');
+
+        // Should redirect to login
+        $this->assertResponseRedirects();
+    }
+
+    public function testIndexPageAccessibleByAdmin(): void
+    {
+        $client = static::createClient();
+        $this->loginAsAdmin($client);
+
+        $client->request('GET', '/system/update-manager');
+
+        $this->assertResponseIsSuccessful();
+    }
+
+    public function testCreateBackupRequiresCsrf(): void
+    {
+        $client = static::createClient();
+        $this->loginAsAdmin($client);
+
+        $client->request('POST', '/system/update-manager/backup', [
+            '_token' => 'invalid',
+        ]);
+
+        // Should redirect with error flash
+        $this->assertResponseRedirects('/system/update-manager');
+    }
+
+    public function testDeleteBackupRequiresCsrf(): void
+    {
+        $client = static::createClient();
+        $this->loginAsAdmin($client);
+
+        $client->request('POST', '/system/update-manager/backup/delete', [
+            '_token' => 'invalid',
+            'filename' => 'test.zip',
+        ]);
+
+        $this->assertResponseRedirects('/system/update-manager');
+    }
+
+    public function testDeleteLogRequiresCsrf(): void
+    {
+        $client = static::createClient();
+        $this->loginAsAdmin($client);
+
+        $client->request('POST', '/system/update-manager/log/delete', [
+            '_token' => 'invalid',
+            'filename' => 'test.log',
+        ]);
+
+        $this->assertResponseRedirects('/system/update-manager');
+    }
+
+    public function testDownloadBackupReturns404ForNonExistent(): void
+    {
+        $client = static::createClient();
+        $this->loginAsAdmin($client);
+
+        $client->request('GET', '/system/update-manager/backup/download/nonexistent.zip');
+
+        $this->assertResponseStatusCodeSame(404);
+    }
+
+    public function testBackupDetailsReturns404ForNonExistent(): void
+    {
+        $client = static::createClient();
+        $this->loginAsAdmin($client);
+
+        $client->request('GET', '/system/update-manager/backup/nonexistent.zip');
+
+        $this->assertResponseStatusCodeSame(404);
+    }
+
+    public function testRestoreBlockedWhenDisabled(): void
+    {
+        $client = static::createClient();
+        $this->loginAsAdmin($client);
+
+        // DISABLE_BACKUP_RESTORE=1 is the default in .env, so this should return 403
+        $client->request('POST', '/system/update-manager/restore', [
+            '_token' => 'invalid',
+            'filename' => 'test.zip',
+        ]);
+
+        $this->assertResponseStatusCodeSame(403);
+    }
+}
diff --git a/tests/Services/System/BackupManagerTest.php b/tests/Services/System/BackupManagerTest.php
index f75ef8f3..9aa92813 100644
--- a/tests/Services/System/BackupManagerTest.php
+++ b/tests/Services/System/BackupManagerTest.php
@@ -82,6 +82,16 @@ final class BackupManagerTest extends KernelTestCase
         $this->assertSame('2.6.0', $matches[2]);
     }
 
+    public function testDeleteBackupReturnsFalseForNonExistentFile(): void
+    {
+        $this->assertFalse($this->backupManager->deleteBackup('non-existent.zip'));
+    }
+
+    public function testDeleteBackupReturnsFalseForNonZipFile(): void
+    {
+        $this->assertFalse($this->backupManager->deleteBackup('not-a-zip.txt'));
+    }
+
     /**
      * Test version parsing with different filename formats.
      */
diff --git a/tests/Services/System/UpdateExecutorTest.php b/tests/Services/System/UpdateExecutorTest.php
index 48cddf8d..c3ea9c1d 100644
--- a/tests/Services/System/UpdateExecutorTest.php
+++ b/tests/Services/System/UpdateExecutorTest.php
@@ -139,6 +139,38 @@ final class UpdateExecutorTest extends KernelTestCase
         $this->assertFalse($this->updateExecutor->isLocked());
     }
 
+    public function testDeleteLogRejectsInvalidFilename(): void
+    {
+        // Path traversal attempts should be rejected
+        $this->assertFalse($this->updateExecutor->deleteLog('../../../etc/passwd'));
+        $this->assertFalse($this->updateExecutor->deleteLog('malicious.txt'));
+        $this->assertFalse($this->updateExecutor->deleteLog(''));
+        // Must start with "update-"
+        $this->assertFalse($this->updateExecutor->deleteLog('backup-v1.0.0.log'));
+    }
+
+    public function testDeleteLogReturnsFalseForNonExistentFile(): void
+    {
+        $this->assertFalse($this->updateExecutor->deleteLog('update-nonexistent-file.log'));
+    }
+
+    public function testDeleteLogDeletesExistingFile(): void
+    {
+        // Create a temporary log file in the update logs directory
+        $projectDir = self::getContainer()->getParameter('kernel.project_dir');
+        $logDir = $projectDir . '/var/update_logs';
+
+        if (!is_dir($logDir)) {
+            mkdir($logDir, 0755, true);
+        }
+
+        $testFile = 'update-test-delete-' . uniqid() . '.log';
+        file_put_contents($logDir . '/' . $testFile, 'test log content');
+
+        $this->assertTrue($this->updateExecutor->deleteLog($testFile));
+        $this->assertFileDoesNotExist($logDir . '/' . $testFile);
+    }
+
     public function testEnableAndDisableMaintenanceMode(): void
     {
         // First, ensure maintenance mode is off