fix: an install rollback drops only the database it created

src/Application/Install/Services/Installer.php

@@ -229,12 +229,18 @@ private function install(): void
             $configData->setDbPass($dbPass);
         }
 
+        $createdDatabase = false;
+
         try {
             // Inside the try: setupDbUser() already created the runtime user, so a
             // failure saving the config must roll it back too
             $this->config->save($configData, false);
 
             $this->databaseSetup->createDatabase($dbUser);
+
+            // Only now may a rollback drop it. Until this line the database is either absent or
+            // somebody else's, and the rollback below used to drop it either way.
+            $createdDatabase = true;
             $this->databaseSetup->createDBStructure();
             $this->databaseSetup->checkConnection();
 
@@ -264,7 +270,7 @@ private function install(): void
             // back over the admin connection
             $this->databaseConnectionData->refreshFromInstallData($this->installData);
 
-            $this->databaseSetup->rollback($dbUser);
+            $this->databaseSetup->rollback($dbUser, $createdDatabase);
 
             throw $e instanceof SPException
                 ? $e

src/Application/Install/Services/MysqlSetup.php

@@ -345,7 +345,7 @@ public function checkDatabaseExists(): bool
      * Best-effort: a rollback failure must never mask the error that triggered
      * it, and one failed statement must not stop the remaining cleanup.
      */
-    public function rollback(?string $dbUser = null): void
+    public function rollback(?string $dbUser = null, bool $createdDatabase = false): void
     {
         try {
             $dbc = $this->dbStorage->getConnectionSimple();
@@ -385,13 +385,32 @@ public function rollback(?string $dbUser = null): void
 
             $this->execBestEffort($dbc, 'SET FOREIGN_KEY_CHECKS = 1');
         } else {
-            $this->execBestEffort(
-                $dbc,
-                sprintf(
-                    'DROP DATABASE IF EXISTS `%s`',
-                    $this->installData->getDbName()
-                )
-            );
+            // Only the database this run created, the way the user below is only dropped when
+            // this run created that.
+            //
+            // The drop used to be unconditional, and nothing established whose database it was.
+            // `install/install` is unauthenticated by necessity, `checkDatabaseAvailability()`
+            // runs before anything is created and `createDatabase()` well after it, with no lock
+            // between them — so two requests (an impatient double-click is enough) both pass the
+            // availability check, the second fails on `CREATE SCHEMA` because the name is now
+            // taken, and its rollback dropped the database the first had just finished installing
+            // into. `config.xml` already said `installed=1`, so the instance claimed to be
+            // installed with no schema behind it, and every later request went to
+            // `error/databaseError`.
+            //
+            // The comment above `checkDatabaseAvailability()` in `Installer::install()` shows the
+            // hazard was understood — "a failure here must not trigger a rollback, which could
+            // otherwise touch pre-existing data". This is that same rule, applied where the
+            // rollback happens rather than where the check does.
+            if ($createdDatabase) {
+                $this->execBestEffort(
+                    $dbc,
+                    sprintf(
+                        'DROP DATABASE IF EXISTS `%s`',
+                        $this->installData->getDbName()
+                    )
+                );
+            }
 
             if ($dbUser) {
                 $this->execBestEffort(

src/Domain/Install/Services/DatabaseSetupService.php

@@ -73,8 +73,9 @@ public function checkConnection(): void;
     /**
      * Roll back the installation in case of failure.
      *
-     * Removes the sysPass database and user. Best-effort: it must never
-     * throw, so it cannot mask the error that triggered it.
+     * Removes what *this* run created, and only that: the user when `$dbUser` names one, and the
+     * database when `$createdDatabase` says this run created it. Best-effort: it must never throw,
+     * so it cannot mask the error that triggered it.
      */
-    public function rollback(?string $dbUser = null): void;
+    public function rollback(?string $dbUser = null, bool $createdDatabase = false): void;
 }

tests/Unit/Application/Install/Services/InstallerTest.php

@@ -395,6 +395,76 @@ public function testHostingModeIsUsed(): void
         $this->assertEquals($params->getDbAdminPass(), $configData->getDbPass());
     }
 
+    /**
+     * A rollback drops the database only when this run was the one that created it.
+     *
+     * `install/install` is unauthenticated by necessity, and the drop used to be unconditional —
+     * nothing established whose database it was. `checkDatabaseAvailability()` runs before anything
+     * is created and `createDatabase()` well after, with no lock between them, so two requests (an
+     * impatient double-click is enough) both pass the availability check; the second fails on
+     * `CREATE SCHEMA` because the name is now taken, and its rollback dropped the database the
+     * first had just finished installing into. `config.xml` already said `installed=1`, so the
+     * instance claimed to be installed with no schema behind it.
+     *
+     * Here the failure happens *before* `createDatabase()` returns, which is exactly the losing
+     * request's shape.
+     *
+     * @throws InvalidArgumentException
+     * @throws SPException
+     */
+    public function testARollbackBeforeTheDatabaseWasCreatedDropsNoDatabase(): void
+    {
+        // Non-hosting on purpose: the unconditional DROP DATABASE was on that branch. The runtime
+        // user is created before the try block, so the pair has to be there to destructure.
+        $this->databaseSetup->method('setupDbUser')->willReturn(['sp_user', 'sp_pass']);
+
+        $this->databaseSetup
+            ->method('createDatabase')
+            ->willThrowException(SPException::error('Error while creating the DB'));
+
+        $this->databaseSetup
+            ->expects($this->once())
+            ->method('rollback')
+            ->with(self::anything(), false);
+
+        $params = $this->getInstallData();
+
+        $installer = $this->getDefaultInstaller();
+
+        $this->expectException(SPException::class);
+
+        $installer->run($params);
+    }
+
+    /**
+     * ...and it does drop it once this run has created it, or a genuinely failed install would
+     * leave its own half-built schema behind and refuse every retry.
+     *
+     * @throws InvalidArgumentException
+     * @throws SPException
+     */
+    public function testARollbackAfterTheDatabaseWasCreatedDropsIt(): void
+    {
+        $this->databaseSetup->method('setupDbUser')->willReturn(['sp_user', 'sp_pass']);
+
+        $this->databaseSetup
+            ->method('createDBStructure')
+            ->willThrowException(SPException::error('Error while creating the DB structure'));
+
+        $this->databaseSetup
+            ->expects($this->once())
+            ->method('rollback')
+            ->with(self::anything(), true);
+
+        $params = $this->getInstallData();
+
+        $installer = $this->getDefaultInstaller();
+
+        $this->expectException(SPException::class);
+
+        $installer->run($params);
+    }
+
     /**
      * @throws InvalidArgumentException
      * @throws SPException

tests/Unit/Application/Install/Services/MySQLTest.php

@@ -554,7 +554,8 @@ public function testRollbackIsSuccessful(): void
 
         $this->pdo->method('quote')->willReturnArgument(0);
 
-        $this->mysqlService->rollback($this->configData->getDbUser());
+        // createdDatabase: this run made it, so the rollback owns it.
+        $this->mysqlService->rollback($this->configData->getDbUser(), true);
     }
 
     public function testRollbackIsSuccessfulWithSameDnsHost(): void
@@ -585,7 +586,8 @@ public function testRollbackIsSuccessfulWithSameDnsHost(): void
 
         $this->pdo->method('quote')->willReturnArgument(0);
 
-        $this->mysqlService->rollback($this->configData->getDbUser());
+        // createdDatabase: this run made it, so the rollback owns it.
+        $this->mysqlService->rollback($this->configData->getDbUser(), true);
     }
 
     public function testRollbackIsSuccessfulWithHostingMode(): void
@@ -619,6 +621,21 @@ public function testRollbackNeverThrows(): void
                   ->willThrowException(new PDOException('test'));
 
         // Best-effort: a rollback failure must not mask the error that triggered it
+        $this->mysqlService->rollback(null, true);
+    }
+
+    /**
+     * A rollback that did not create the database does not drop one.
+     *
+     * `install/install` is unauthenticated by necessity, and the drop used to be unconditional —
+     * nothing established whose database it was. Two requests both pass
+     * `checkDatabaseAvailability()` before either creates anything, and the one that then loses the
+     * `CREATE SCHEMA` race used to drop the database the winner had just installed into.
+     */
+    public function testRollbackDropsNoDatabaseItDidNotCreate(): void
+    {
+        $this->pdo->expects(self::never())->method('exec');
+
         $this->mysqlService->rollback();
     }