Skip to content

fix(versions): preserve version encrypted version before rollback - #61562

Closed
leftybournes wants to merge 1 commit into
masterfrom
leftybournes/fix/versions-restore-with-encryption
Closed

fix(versions): preserve version encrypted version before rollback#61562
leftybournes wants to merge 1 commit into
masterfrom
leftybournes/fix/versions-restore-with-encryption

Conversation

@leftybournes

Copy link
Copy Markdown
Member

Summary

While rolling back, the created file version's encrypted version is set to 1 when it is actually higher, causing it to appear corrupted when restoring to it. This preserves the encrypted version for the created file version to prevent "corruption" from happening.

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Signed-off-by: Kent Delante <kent@delante.me>
@leftybournes leftybournes added this to the Nextcloud 35 milestone Jun 24, 2026
@leftybournes
leftybournes requested a review from a team as a code owner June 24, 2026 09:09
@leftybournes
leftybournes requested review from Altahrim, ArtificialOwl, come-nc and provokateurin and removed request for a team June 24, 2026 09:09
$versionCache->update(
$versionFileInfo->getId(), [
'encrypted' => $encryptedVersion,
'encryptedVersion' => $encryptedVersion,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

encrypted should probably not get set or at least not to the version?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tested it, just setting encryptedVersion "corrupts" the version but also setting encrypted fixes the issue.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we already do some of this below too, but I feel like we should be letting the Encryption storage wrapper deal with these matters -- e.g.

private function copyBetweenStorage(
Storage\IStorage $sourceStorage,
string $sourceInternalPath,
string $targetInternalPath,
bool $preserveMtime,
bool $isRename,
): bool {
// for versions we have nothing to do, because versions should always use the
// key from the original file. Just create a 1:1 copy and done
if ($this->isVersion($targetInternalPath)
|| $this->isVersion($sourceInternalPath)) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshtrichards I dug into it further and it seems it's already handled in the Encryption wrapper.

// in case of a rename we need to manipulate the source cache because
// this information will be kept for the new target
if ($isRename) {
$sourceStorage->getCache()->put($sourceInternalPath, $cacheInformation);
} else {
$this->getCache()->put($targetInternalPath, $cacheInformation);
}

The problem is the encryptedVersion value gets overwritten when a new version is created. I noticed a sequence where Encryption sets the encryptedVersion and then View calls a copyUpdate which overwrites the encryptedVersion value to 1 which is why this workaround is needed.

if ($result) {
$this->copyUpdate($storage1, $storage2, $internalPath1, $internalPath2);
}

@joshtrichards joshtrichards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is begging for test coverage. :-)

@leftybournes

Copy link
Copy Markdown
Member Author

After doing some further digging, I found the root cause and opened a new pull request. I'll close this if it's merged.

@leftybournes

Copy link
Copy Markdown
Member Author

Closing this in favor of #62255

@leftybournes
leftybournes deleted the leftybournes/fix/versions-restore-with-encryption branch July 20, 2026 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Encrypted files become corrupted after restoring a version

3 participants