From 048489ea1ed23b0351e2782fbbb219c665893eee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20M=C3=BCller?=
 <1005065+DeepDiver1975@users.noreply.github.com>
Date: Sun, 21 Jun 2026 23:11:55 +0200
Subject: [PATCH] fix(trashbin): don't overwrite existing file when restoring
 to a target
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Trashbin::restore() resolves a collision-safe unique name only on the
default path (restore to original location, $targetLocation === null).
When an explicit target is given — the WebDAV "restore to a different
place" path — it skipped that step and called View::rename() directly,
silently overwriting any file already at the target and causing data
loss (issue #35974).

Add a guard before the rename: if the explicit target already exists,
refuse the restore and return false instead of clobbering it. The caller
contract (bool) is preserved and propagates through TrashBinManager and
AbstractTrashBinNode. The default null-target path already resolves to a
unique, non-existing name, so normal restores are unaffected.

Adds a regression test: restoring onto an existing target returns false
and leaves the existing file's content untouched.

Fixes #35974

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
---
 apps/files_trashbin/lib/Trashbin.php       | 11 ++++++
 apps/files_trashbin/tests/TrashbinTest.php | 41 ++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/apps/files_trashbin/lib/Trashbin.php b/apps/files_trashbin/lib/Trashbin.php
index 189728cd587f..8d414764a504 100644
--- a/apps/files_trashbin/lib/Trashbin.php
+++ b/apps/files_trashbin/lib/Trashbin.php
@@ -581,6 +581,17 @@ public static function restore($filename, $targetLocation = null) {
 		if (!$view->file_exists($source)) {
 			return false;
 		}
+
+		// Data-loss guard for issue #35974: when an explicit target location is
+		// given (e.g. WebDAV "restore to a different place"), the unique-name
+		// logic above is skipped. Refuse to overwrite an already existing target
+		// instead of silently clobbering it; the caller is informed via the
+		// boolean return value. The default (null target) path already resolves
+		// to a unique, non-existing name, so this never blocks a normal restore.
+		if ($view->file_exists($target)) {
+			return false;
+		}
+
 		$mtime = $view->filemtime($source);
 
 		// restore file
diff --git a/apps/files_trashbin/tests/TrashbinTest.php b/apps/files_trashbin/tests/TrashbinTest.php
index e4821de405f3..cdb8c9cf43a8 100644
--- a/apps/files_trashbin/tests/TrashbinTest.php
+++ b/apps/files_trashbin/tests/TrashbinTest.php
@@ -455,6 +455,47 @@ public function testRestoreFileDoesNotOverwriteExistingInSubfolder() {
 		$this->assertEquals('foo', $restoredFile->getContent());
 	}
 
+	/**
+	 * Test restoring a file to an explicit target location that already holds a
+	 * different file. Regression test for issue #35974: the restore must NOT
+	 * overwrite the existing target (which would cause data loss). It must fail
+	 * (return false) and leave the existing target file untouched.
+	 */
+	public function testRestoreToExplicitTargetDoesNotOverwriteExisting() {
+		$userFolder = \OC::$server->getUserFolder();
+		$file = $userFolder->newFile('file1.txt');
+		$file->putContent('foo');
+
+		$this->assertTrue($userFolder->nodeExists('file1.txt'));
+
+		$file->delete();
+
+		$this->assertFalse($userFolder->nodeExists('file1.txt'));
+
+		$filesInTrash = Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'mtime');
+		$this->assertCount(1, $filesInTrash);
+
+		/** @var FileInfo */
+		$trashedFile = $filesInTrash[0];
+
+		// create a DIFFERENT existing file at the explicit target location
+		$existingTarget = $userFolder->newFile('existing.txt');
+		$existingTarget->putContent('do-not-overwrite');
+
+		// restore to the explicit target that already exists: must fail
+		$this->assertFalse(
+			Trashbin::restore(
+				'file1.txt.d' . $trashedFile->getMtime(),
+				'existing.txt'
+			)
+		);
+
+		// the existing target file must be untouched (no data loss)
+		/** @var File $stillThere */
+		$stillThere = $userFolder->get('existing.txt');
+		$this->assertEquals('do-not-overwrite', $stillThere->getContent());
+	}
+
 	/**
 	 * Test restoring a nonexistent file from trashbin, returns false
 	 */