fix(encryption): keep the encrypted version the copy was written with

apps/encryption/tests/EncryptedCopyTest.php

@@ -0,0 +1,139 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\encryption\tests;
+
+use OC\Files\Storage\Temporary;
+use OC\Files\View;
+use OCA\Encryption\KeyManager;
+use OCP\Server;
+use Test\TestCase;
+use Test\Traits\EncryptionTrait;
+use Test\Traits\MountProviderTrait;
+use Test\Traits\UserTrait;
+
+/**
+ * A copy re-encrypts the target, so its blocks are signed with the version that
+ * follows the version of the file that was overwritten - neither with the source's
+ * version nor with version 1. Reading the target back only works if the cache
+ * entry holds that same version.
+ */
+#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
+class EncryptedCopyTest extends TestCase {
+	use MountProviderTrait;
+	use EncryptionTrait;
+	use UserTrait;
+
+	private function setUpView(): View {
+		Server::get(KeyManager::class)->validateMasterKey();
+		Server::get(KeyManager::class)->validateShareKey();
+		$this->createUser('test1', 'test2');
+		$this->setupForUser('test1', 'test2');
+		$this->registerMount('test1', new Temporary(), '/test1/files/other');
+		$this->loginWithEncryption('test1');
+
+		return new View('/test1/files');
+	}
+
+	/**
+	 * The version the target ends up at depends on the storage: a copy unlinks the
+	 * target first, which keeps the cache entry - and with it the version to bump -
+	 * on a local storage but drops it on an object store, where the copy therefore
+	 * starts over at version 1. Reading the target back is what shows that the
+	 * recorded version is the one its blocks were signed with.
+	 */
+	public function testCopyOverExistingFile(): void {
+		$view = $this->setUpView();
+		$source = str_repeat('a', 20000);
+
+		$view->file_put_contents('source.bin', $source);
+		$view->file_put_contents('target.bin', str_repeat('b', 20000));
+
+		$this->assertTrue($view->copy('source.bin', 'target.bin'));
+
+		$this->assertEquals($source, $view->file_get_contents('target.bin'));
+	}
+
+	/**
+	 * Every write bumps the version of the source, while the copy of it starts over
+	 * at the version of the target.
+	 */
+	public function testCopyFileWrittenSeveralTimes(): void {
+		$view = $this->setUpView();
+		$source = str_repeat('c', 20000);
+
+		$view->file_put_contents('source.bin', str_repeat('a', 20000));
+		$view->file_put_contents('source.bin', str_repeat('b', 20000));
+		$view->file_put_contents('source.bin', $source);
+
+		$this->assertTrue($view->copy('source.bin', 'target.bin'));
+
+		$this->assertEquals($source, $view->file_get_contents('target.bin'));
+	}
+
+	public function testCopyOverExistingFileWrittenSeveralTimes(): void {
+		$view = $this->setUpView();
+		$source = str_repeat('a', 20000);
+
+		$view->file_put_contents('source.bin', $source);
+		$view->file_put_contents('target.bin', str_repeat('b', 20000));
+		$view->file_put_contents('target.bin', str_repeat('c', 20000));
+
+		$this->assertTrue($view->copy('source.bin', 'target.bin'));
+
+		$this->assertEquals($source, $view->file_get_contents('target.bin'));
+	}
+
+	public function testCopyFolderOverExistingFolder(): void {
+		$view = $this->setUpView();
+		$source = str_repeat('a', 20000);
+
+		$view->mkdir('source');
+		$view->file_put_contents('source/file.bin', $source);
+		$view->mkdir('target');
+		$view->file_put_contents('target/file.bin', str_repeat('b', 20000));
+
+		$this->assertTrue($view->copy('source', 'target'));
+
+		$this->assertEquals($source, $view->file_get_contents('target/file.bin'));
+	}
+
+	public function testMoveOverExistingFileOnAnotherStorage(): void {
+		$view = $this->setUpView();
+		$source = str_repeat('a', 20000);
+
+		$view->file_put_contents('source.bin', $source);
+		$view->file_put_contents('other/target.bin', str_repeat('b', 20000));
+
+		$this->assertTrue($view->rename('source.bin', 'other/target.bin'));
+
+		$this->assertEquals($source, $view->file_get_contents('other/target.bin'));
+	}
+
+	/**
+	 * The target is not unlinked when it lives on another storage, so it keeps its
+	 * cache entry and the copy is signed with the version that follows the one of
+	 * the file it overwrites.
+	 */
+	public function testCopyOverExistingFileOnAnotherStorage(): void {
+		$view = $this->setUpView();
+		$source = str_repeat('a', 20000);
+
+		$view->file_put_contents('source.bin', $source);
+		$view->file_put_contents('other/target.bin', str_repeat('b', 20000));
+
+		$this->assertTrue($view->copy('source.bin', 'other/target.bin'));
+
+		$this->assertEquals($source, $view->file_get_contents('other/target.bin'));
+		$this->assertEquals(
+			2,
+			$view->getFileInfo('other/target.bin')->getEncryptedVersion(),
+			'the version of the overwritten file was not bumped'
+		);
+	}
+}

build/integration/encryption_features/encryption.feature

@@ -29,6 +29,28 @@ Feature: encryption
     When Downloading file "/copy.bin"
     Then the HTTP status code should be "200"
 
+  Scenario: Copy a file over an existing file
+    Given user "user0" exists
+    And As an "user0"
+    And User "user0" uploads file with content "the source content" to "/source.txt"
+    And User "user0" uploads file with content "the target content" to "/target.txt"
+    When User "user0" copies file "/source.txt" to "/target.txt"
+    Then the HTTP status code should be "204"
+    When Downloading file "/target.txt"
+    Then the HTTP status code should be "200"
+    And Downloaded content should be "the source content"
+
+  Scenario: Copy a file that was written several times
+    Given user "user0" exists
+    And As an "user0"
+    And User "user0" uploads file with content "the first content" to "/source.txt"
+    And User "user0" uploads file with content "the second content" to "/source.txt"
+    When User "user0" copies file "/source.txt" to "/copy.txt"
+    Then the HTTP status code should be "201"
+    When Downloading file "/copy.txt"
+    Then the HTTP status code should be "200"
+    And Downloaded content should be "the second content"
+
   # With "part_file_in_storage" disabled the part file is written to the user
   # home while the target lives on another storage, so the upload has to read the
   # part file back to move it over. A part file never has a file cache entry, so

lib/private/Files/Cache/Cache.php

@@ -1281,6 +1281,14 @@ public function copyFromCache(ICache $sourceCache, ICacheEntry $sourceEntry, str
 			// normalizeData() prefers 'encryptedVersion' over 'encrypted' when both are
 			// set, so it has to be cleared too or the mark above gets ignored
 			unset($data['encryptedVersion']);
+		} elseif (isset($data['encryptedVersion'])) {
+			// The storage re-encrypts the content it writes to the target, so the target
+			// is at its own version - the one recorded for it while it was written - and
+			// not at the version of the source.
+			$targetEntry = $this->get($targetPath);
+			if ($targetEntry !== false && !empty($targetEntry['encryptedVersion'])) {
+				$data['encryptedVersion'] = $targetEntry['encryptedVersion'];
+			}
 		}
 
 		$fileId = $this->put($targetPath, $data);

lib/private/Files/Storage/Wrapper/Encryption.php

@@ -635,21 +635,34 @@ private function updateEncryptedVersion(
 
 			// Rename of the cache already happened, so we do the cleanup on the target
 			if ($sourceCacheEntry === false && $targetCacheEntry !== false) {
-				$encryptedVersion = $targetCacheEntry['encryptedVersion'];
 				$isRename = false;
-			} elseif ($sourceCacheEntry === false) {
-				// a file that is not in the file cache, e.g. a part file, is at version 1
-				$encryptedVersion = 1;
+			}
+
+			if ($keepEncryptionVersion) {
+				// a 1:1 copy reuses the keys and the ciphertext of the source, so the
+				// target stays at the version of the source
+				if ($sourceCacheEntry !== false) {
+					$encryptedVersion = (int)($sourceCacheEntry['encryptedVersion'] ?? 0);
+				} elseif ($targetCacheEntry !== false) {
+					$encryptedVersion = (int)($targetCacheEntry['encryptedVersion'] ?? 0);
+				} else {
+					// a file that is not in the file cache, e.g. a part file, is at version 1
+					$encryptedVersion = 1;
+				}
 			} else {
-				$encryptedVersion = $sourceCacheEntry['encryptedVersion'];
+				// The target was written through the encryption stream, which signs the
+				// blocks with the version that follows the version of the file they
+				// replaced and records it on the target's cache entry. A target that has
+				// no cache entry was written at version 1.
+				$encryptedVersion = $targetCacheEntry === false ? 1 : (int)($targetCacheEntry['encryptedVersion'] ?? 0);
 			}
 
 			// In case of a move operation from an unencrypted to an encrypted
 			// storage the old encrypted version would stay with "0" while the
 			// correct value would be "1". Thus we manually set the value to "1"
 			// for those cases.
 			// See also https://github.com/owncloud/core/issues/23078
-			if ($encryptedVersion === 0 || !$keepEncryptionVersion) {
+			if ($encryptedVersion === 0) {
 				$encryptedVersion = 1;
 			}
 

tests/lib/Files/Storage/Wrapper/EncryptionTest.php

@@ -935,6 +935,75 @@ public static function dataCopyBetweenStorage(): array {
 		];
 	}
 
+	public static function dataUpdateEncryptedVersion(): array {
+		return [
+			// the target is written through the encryption stream, which signs its blocks
+			// with the version that follows the version of the file they replace
+			'copy onto an existing file' => [['encryptedVersion' => 4], ['encryptedVersion' => 3], false, 3],
+			'copy onto a new file' => [['encryptedVersion' => 4], false, false, 1],
+			'copy onto a file that is not encrypted yet' => [['encryptedVersion' => 4], ['encryptedVersion' => 0], false, 1],
+			// a 1:1 copy reuses the keys and the ciphertext of the source
+			'1:1 copy' => [['encryptedVersion' => 5], false, true, 5],
+			'1:1 copy of a file that is not encrypted yet' => [['encryptedVersion' => 0], false, true, 1],
+		];
+	}
+
+	#[\PHPUnit\Framework\Attributes\DataProvider('dataUpdateEncryptedVersion')]
+	public function testUpdateEncryptedVersion(
+		array|false $sourceCacheEntry,
+		array|false $targetCacheEntry,
+		bool $keepEncryptionVersion,
+		int $expectedVersion,
+	): void {
+		$sourceCache = $this->createMock(ICache::class);
+		$sourceCache->method('get')
+			->with('source.txt')
+			->willReturn($sourceCacheEntry);
+		$sourceStorage = $this->createMock(\OC\Files\Storage\Storage::class);
+		$sourceStorage->method('getCache')
+			->willReturn($sourceCache);
+
+		$targetCache = $this->createMock(ICache::class);
+		$targetCache->method('get')
+			->with('target.txt')
+			->willReturn($targetCacheEntry);
+		$targetCache->expects($this->once())
+			->method('put')
+			->with('target.txt', ['encrypted' => true, 'encryptedVersion' => $expectedVersion]);
+
+		$instance = $this->getMockBuilder(Encryption::class)
+			->setConstructorArgs(
+				[
+					[
+						'storage' => $this->sourceStorage,
+						'root' => 'foo',
+						'mountPoint' => '/',
+						'mount' => $this->mount
+					],
+					$this->encryptionManager,
+					$this->util,
+					$this->logger,
+					$this->file,
+					null,
+					$this->keyStore,
+					$this->mountManager,
+					$this->arrayCache
+				]
+			)
+			->onlyMethods(['getCache', 'getEncryptionModule'])
+			->getMock();
+		$instance->method('getCache')->willReturn($targetCache);
+		$instance->method('getEncryptionModule')->willReturn($this->encryptionModule);
+
+		$this->encryptionManager->expects($this->any())
+			->method('isEnabled')
+			->willReturn(true);
+		global $mockedMountPointEncryptionEnabled;
+		$mockedMountPointEncryptionEnabled = true;
+
+		$this->invokePrivate($instance, 'updateEncryptedVersion', [$sourceStorage, 'source.txt', 'target.txt', false, $keepEncryptionVersion]);
+	}
+
 	public function testCopyBetweenStorageMinimumEncryptedVersion(): void {
 		$storage2 = $this->createMock(\OC\Files\Storage\Storage::class);