Skip to content

fix(encryption): refactor/improve DecryptAll/EncryptAll robustness/error handling #57506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
joshtrichards wants to merge 5 commits into
base: master
Choose a base branch
from
+282 −118
Draft

fix(encryption): refactor/improve DecryptAll/EncryptAll robustness/error handling #57506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
81c52d5
refactor(encryption): improve DecryptAll robustness/error handling
joshtrichards Jan 13, 2026
03f2aa3
chore(encryption): tidy up EncryptAll / unify more with DecryptAll
joshtrichards Jan 13, 2026
f64dfd1
chore: add import (FileInfo)
joshtrichards Jan 13, 2026
da7a0f9
test: new lib/Encryption/DecryptAllTest
joshtrichards Jan 13, 2026
0df02e1
chore: fixup DecryptAll.php
joshtrichards Jan 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 40 additions & 35 deletions apps/encryption/lib/Crypto/EncryptAll.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

namespace OCA\Encryption\Crypto;

use OC\Encryption\Exceptions\DecryptionFailedException;
use OC\Encryption\Exceptions\EncryptionFailedException;
use OC\Files\SetupManager;
use OC\Files\View;
use OCA\Encryption\KeyManager;
Expand All @@ -33,6 +33,9 @@
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Question\ConfirmationQuestion;

/**
* Handles bulk encryption of files for users.
*/
class EncryptAll {

/** @var array<string, array{password: string, user: IUser}> $userCache store one time passwords for the users */
Expand Down Expand Up @@ -168,7 +171,10 @@ protected function encryptAllUserFilesWithMasterKey(ProgressBar $progress): void
}

/**
* encrypt files from the given user
* Encrypt all files from the given user.
*
* Recursively traverses the user's files directory, skipping files and folders not owned by the user,
* and attempts to encrypt each file.
*/
protected function encryptUsersFiles(IUser $user, ProgressBar $progress, string $userCount): void {
$this->setupUserFileSystem($user);
Expand All @@ -178,64 +184,63 @@ protected function encryptUsersFiles(IUser $user, ProgressBar $progress, string

while ($root = array_pop($directories)) {
$content = $this->rootView->getDirectoryContent($root);
/** @var FileInfo $file */
foreach ($content as $file) {
$path = $root . '/' . $file->getName();
if ($file->isShared()) {
$progress->setMessage("Skip shared file/folder $path");

if ($file->getOwner() !== $uid) {
$progress->setMessage("Skipping shared/unowned file/folder $path");
$progress->advance();
continue;
} elseif ($file->getType() === FileInfo::TYPE_FOLDER) {
}

if ($file->getType() === FileInfo::TYPE_FOLDER) {
$directories[] = $path;
continue;
} else {
$progress->setMessage("encrypt files for user $userCount: $path");
$progress->advance();
try {
if ($this->encryptFile($file, $path) === false) {
$progress->setMessage("encrypt files for user $userCount: $path (already encrypted)");
$progress->advance();
}
} catch (\Exception $e) {
$progress->setMessage("Failed to encrypt path $path: " . $e->getMessage());
}

$progress->setMessage("Encrypting file for user $userCount: $path");
$progress->advance();

try {
if ($this->encryptFile($file, $path) === false) {
$progress->setMessage("Skipping already encrypted file $path for user $userCount");
$progress->advance();
$this->logger->error(
'Failed to encrypt path {path}',
[
'user' => $uid,
'path' => $path,
'exception' => $e,
]
);
}
} catch (\Exception $e) {
$progress->setMessage("Failed to encrypt path $path: " . $e->getMessage());
$progress->advance();
$this->logger->error('Failed to encrypt path {path}', [ 'user' => $uid, 'path' => $path, 'exception' => $e, ]);
}
}
}
}

protected function encryptFile(FileInfo $fileInfo, string $path): bool {
// skip already encrypted files
if ($fileInfo->isEncrypted()) {
return true;
return false;
}

$source = $path;
$target = $path . '.encrypted.' . time();

try {
$copySuccess = $this->rootView->copy($source, $target);
if ($copySuccess === false) {
/* Copy failed, abort */
if ($this->rootView->file_exists($target)) {
$this->rootView->unlink($target);
}
throw new \Exception('Copy failed for ' . $source);
if ($this->rootView->copy($source, $target) === false) {
throw new EncryptionFailedException("Failed to copy $source -> $target");
}
$this->rootView->rename($target, $source);
} catch (DecryptionFailedException $e) {

if ($this->rootView->rename($target, $source) === false) {
throw new EncryptionFailedException("Failed to rename $target -> $source");
}
} catch (\Exception $e) {
if ($this->rootView->file_exists($target)) {
$this->logger->debug(
"Cleaning up failed temp file $target after encryption exception",
[ 'user' => $fileInfo->getOwner(), 'path' => $path, ]
);
$this->rootView->unlink($target);
}
return false;
throw $e;
}

return true;
Expand Down
129 changes: 80 additions & 49 deletions lib/private/Encryption/DecryptAll.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,33 @@
namespace OC\Encryption;

use OC\Encryption\Exceptions\DecryptionFailedException;
use OC\Files\FileInfo;
use OC\Files\View;
use OCP\Encryption\IEncryptionModule;
use OCP\Encryption\IManager;
use OCP\IUserManager;
use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Helper\ProgressBar;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

/**
* DecryptAll handles bulk decryption of files for users.
*/
class DecryptAll {
/** @var array<string,list<string>> files which couldn't be decrypted */
protected array $failed = [];

protected readonly LoggerInterface $logger;

public function __construct(
protected IManager $encryptionManager,
protected IUserManager $userManager,
protected View $rootView,
protected readonly IManager $encryptionManager,
protected readonly IUserManager $userManager,
protected readonly View $rootView,
) {
// TODO: Inject LoggerInterface
$this->logger = \OC::$server->get(LoggerInterface::class);
// TODO: Inject SetupManager
}

/**
Expand All @@ -51,6 +61,7 @@ public function decryptAll(InputInterface $input, OutputInterface $output, strin
$this->failed = [];
$this->decryptAllUsersFiles($output, $user);

// TODO: Drop below output; maybe still use $this->failed to return false (if we can't switch to void)
/** @psalm-suppress RedundantCondition $this->failed is modified by decryptAllUsersFiles, not clear why psalm fails to see it */
if (empty($this->failed)) {
$output->writeln('all files could be decrypted successfully!');
Expand Down Expand Up @@ -91,7 +102,7 @@ protected function prepareEncryptionModules(InputInterface $input, OutputInterfa
}

/**
* iterate over all user and encrypt their files
* iterate over all user and decrypt their files
*
* @param string $user which users files should be decrypted, default = all users
*/
Expand Down Expand Up @@ -129,7 +140,7 @@ protected function decryptAllUsersFiles(OutputInterface $output, string $user =
$progress = new ProgressBar($output);
$progress->setFormat(" %message% \n [%bar%]");
$progress->start();
$progress->setMessage('starting to decrypt files...');
$progress->setMessage('Decrypting files...');
$progress->advance();

$numberOfUsers = count($userList);
Expand All @@ -140,94 +151,114 @@ protected function decryptAllUsersFiles(OutputInterface $output, string $user =
$userNo++;
}

$progress->setMessage('starting to decrypt files... finished');
$progress->setMessage('Decrypting files... finished');
$progress->finish();

$output->writeln("\n\n");
}

/**
* encrypt files from the given user
*/
protected function decryptUsersFiles(string $uid, ProgressBar $progress, string $userCount): void {
/**
* Decrypt all files as the given user.
*
* Recursively traverses the user's files directory, skipping files and folders not owned by the user,
* and attempts to decrypt each file.
*/
protected function decryptUsersFiles(string $uid, ProgressBar $progress, string $userCount): void {
$this->setupUserFS($uid);
$directories = [];
$directories[] = '/' . $uid . '/files';

while ($root = array_pop($directories)) {
$content = $this->rootView->getDirectoryContent($root);
/** @var FileInfo $file */
foreach ($content as $file) {
// only decrypt files owned by the user
if ($file->getStorage()->instanceOfStorage('OCA\Files_Sharing\SharedStorage')) {
$path = $root . '/' . $file->getName();

if ($file->getOwner() !== $uid) {
$progress->setMessage("Skipping shared/unowned file/folder $path");
$progress->advance();
continue;
}
$path = $root . '/' . $file['name'];
if ($this->rootView->is_dir($path)) {

if ($file->getType() === FileInfo::TYPE_FOLDER) {
$directories[] = $path;
continue;
} else {
try {
$progress->setMessage("decrypt files for user $userCount: $path");
}

$progress->setMessage("Decrypting file for user $userCount: $path");
$progress->advance();

try {
if ($this->decryptFile($path) === false) {
$progress->setMessage("Skipping already decrypted file $path for user $userCount");
$progress->advance();
if ($file->isEncrypted() === false) {
$progress->setMessage("decrypt files for user $userCount: $path (already decrypted)");
$progress->advance();
} else {
if ($this->decryptFile($path) === false) {
$progress->setMessage("decrypt files for user $userCount: $path (already decrypted)");
$progress->advance();
}
}
} catch (\Exception $e) {
if (isset($this->failed[$uid])) {
$this->failed[$uid][] = $path;
} else {
$this->failed[$uid] = [$path];
}
}
} catch (\Exception $e) {
$progress->setMessage("Failed to decrypt path $path: " . $e->getMessage());
$progress->advance();
$this->logger->error('Failed to decrypt path {path}', [ 'user' => $uid, 'path' => $path, 'exception' => $e, ]);
// TODO: we can probably drop this since we're now outputting above like we do in EncryptAll
if (isset($this->failed[$uid])) {
$this->failed[$uid][] = $path;
} else {
$this->failed[$uid] = [$path];
}
}
}
}
}

/**
* encrypt file
* Attempt to decrypt a single file.
* @param string $path The full filesystem path to the file.
*
* @throws DecryptionFailedException If file copy or rename fails during decryption.
* @throws \RuntimeException If file info cannot be retrieved or touch fails.
*
* @return bool True if decryption succeeded, false if file is already decrypted.
*/
protected function decryptFile(string $path): bool {
// skip already decrypted files
$fileInfo = $this->rootView->getFileInfo($path);
if ($fileInfo !== false && !$fileInfo->isEncrypted()) {
return true;

if ($fileInfo === false) {
throw new \RuntimeException("Could not retrieve file info for $path");
}

if (!$fileInfo->isEncrypted()) {
return false;
}

$source = $path;
$target = $path . '.decrypted.' . $this->getTimestamp();
$target = $path . '.decrypted.' . time();

try {
$this->rootView->copy($source, $target);
$this->rootView->touch($target, $fileInfo->getMTime());
$this->rootView->rename($target, $source);
} catch (DecryptionFailedException $e) {
if ($this->rootView->copy($source, $target) === false) {
throw new DecryptionFailedException("Failed to copy $source -> $target");
}

if ($this->rootView->touch($target, $fileInfo->getMTime()) === false) {
throw new \RuntimeException("Failed to update mtime for $target");
}

if ($this->rootView->rename($target, $source) === false) {
throw new DecryptionFailedException("Failed to rename $target -> $source");
}
} catch (\Exception $e) {
if ($this->rootView->file_exists($target)) {
$this->logger->debug("Cleaning up failed temp file $target after decryption exception", [ 'path' => $path, ]);
$this->rootView->unlink($target);
}
return false;
throw $e;
}

return true;
}

/**
* get current timestamp
*/
protected function getTimestamp(): int {
return time();
}

/**
* setup user file system
*/
protected function setupUserFS(string $uid): void {
// TODO: Refactor to use injected SetupManager (like EncryptAll does) + the IUser objeect
\OC_Util::tearDownFS();
\OC_Util::setupFS($uid);
}
Expand Down
Loading
Loading