fix(installer): validate app archive before deleting installed app on update

Installer::updateApp() removed the currently-installed app directory
(`rmdirr($basedir)`) before copying the new version from the extracted
archive, and — unlike installApp() — never checked that the archive
actually contained a directory named after the app id. An invalid
tarball (top-level directory not matching the app id) therefore deleted
the installed app and then tried to copy from a non-existent source,
leaving the app broken and surfacing a misleading error instead of a
clear message (issue #34669).

Mirror installApp()'s guard in updateApp(): compute the expected app
directory inside the extract dir and, BEFORE the destructive removal of
the installed app, verify it exists. If not, clean up the extract dir
and throw the same translated "Archive does not contain a directory
named %s" error installApp() uses. The installed app is now left intact
on invalid input, and the success path for valid archives is unchanged.

Adds a regression test (with an invalid-archive fixture) asserting the
clear error is thrown and the previously-installed app is not deleted.

Fixes #34669

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>

Author: DeepDiver1975

lib/private/Installer.php

@@ -214,6 +214,8 @@ public static function isInstalled($app) {
 	 * "\OC::$server->getAppConfig()->getValue($appid, 'installed_version')"
 	 */
 	public static function updateApp($info= [], $isShipped=false) {
+		$l = \OC::$server->getL10N('lib');
+
 		list($extractDir, $path) = self::downloadApp($info);
 		$info = self::checkAppsIntegrity($info, $extractDir, $path, $isShipped);
 
@@ -230,16 +232,25 @@ public static function updateApp($info= [], $isShipped=false) {
 			$basedir = $currentDir;
 		}
 
-		if (\is_dir($basedir)) {
-			OC_Helper::rmdirr($basedir);
-		}
-
 		$appInExtractDir = $extractDir;
 		if (\substr($extractDir, -1) !== '/') {
 			$appInExtractDir .= '/';
 		}
 
 		$appInExtractDir .= $info['id'];
+
+		// Verify the archive actually contains the app directory before
+		// removing the currently installed app, otherwise an invalid archive
+		// would leave the user without the previously installed app.
+		if (!\is_dir($appInExtractDir)) {
+			OC_Helper::rmdirr($extractDir);
+			throw new \Exception($l->t("Archive does not contain a directory named %s", $info['id']));
+		}
+
+		if (\is_dir($basedir)) {
+			OC_Helper::rmdirr($basedir);
+		}
+
 		OC_Helper::copyr($appInExtractDir, $basedir);
 		OC_Helper::rmdirr($extractDir);
 

tests/data/testapp_invalid.zip

@@ -91,6 +91,57 @@ public function testUpdateApp() {
 		$this->assertNotEquals($oldVersionNumber, $newVersionNumber);
 	}
 
+	/**
+	 * Tests that updating with an archive whose top-level directory is not
+	 * named after the app id fails with a clear error message and, crucially,
+	 * does not destroy the already-installed app.
+	 */
+	public function testUpdateWithInvalidArchiveKeepsInstalledApp() {
+		// Install the valid app first
+		$pathOfOldTestApp = __DIR__ . '/../data/testapp.zip';
+		$oldTmp = \OC::$server->getTempManager()->getTemporaryFile('.zip');
+		\OC_Helper::copyr($pathOfOldTestApp, $oldTmp);
+		$oldData = [
+			'path' => $oldTmp,
+			'source' => 'path',
+			'appdata' => [
+				'id' => 'testapp',
+				'level' => 100,
+			]
+		];
+		Installer::installApp($oldData);
+		$this->assertTrue(Installer::isInstalled(self::$appid));
+		$appPath = \OC_App::getAppPath(self::$appid);
+		$this->assertNotFalse($appPath);
+		$this->assertTrue(\is_dir($appPath));
+
+		// Attempt to update with an invalid archive: its top-level directory is
+		// "wrongname" although info.xml declares the id "testapp".
+		$pathOfInvalidTestApp = __DIR__ . '/../data/testapp_invalid.zip';
+		$invalidTmp = \OC::$server->getTempManager()->getTemporaryFile('.zip');
+		\OC_Helper::copyr($pathOfInvalidTestApp, $invalidTmp);
+		$invalidData = [
+			'path' => $invalidTmp,
+			'source' => 'path',
+			'appdata' => [
+				'id' => 'testapp',
+				'level' => 100,
+			]
+		];
+
+		$thrown = false;
+		try {
+			Installer::updateApp($invalidData);
+		} catch (\Exception $e) {
+			$thrown = true;
+			$this->assertStringStartsWith('Archive does not contain a directory named', $e->getMessage());
+		}
+		$this->assertTrue($thrown, 'updateApp() should throw for an invalid archive');
+
+		// The previously installed app must still be present and untouched.
+		$this->assertTrue(\is_dir($appPath), 'The installed app directory must not be deleted on invalid update');
+	}
+
 	/**
 	 * Tests that update is installed into writable app dir if the original app dir is not writable
 	 */