Skip to content

fix(encryption): Refactor EncryptionWrapper with HomeMountPoint support#60001

Open
cuppett wants to merge 3 commits intonextcloud:masterfrom
cuppett:refactor/encryption-wrapper-homemount-support
Open

fix(encryption): Refactor EncryptionWrapper with HomeMountPoint support#60001
cuppett wants to merge 3 commits intonextcloud:masterfrom
cuppett:refactor/encryption-wrapper-homemount-support

Conversation

@cuppett
Copy link
Copy Markdown
Contributor

@cuppett cuppett commented Apr 29, 2026

Summary

Rewrite conditional flow to use early-return guards: skip IDisableEncryptionStorage, skip the root mount, respect encryptHomeStorage for HomeMountPoints. Uses IAppConfig for the encryptHomeStorage setting with a legacy string fallback for the upgrade window.

Spawned from #57279

Checklist

AI (if applicable)

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

@cuppett cuppett requested a review from a team as a code owner April 29, 2026 23:49
@cuppett cuppett requested review from Altahrim, ArtificialOwl, leftybournes and salmart-dev and removed request for a team April 29, 2026 23:49
@cuppett cuppett added this to the Nextcloud 34 milestone Apr 29, 2026
@cuppett cuppett added 3. to review Waiting for reviews feature: encryption (client-side) ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Apr 29, 2026
Copy link
Copy Markdown
Contributor Author

@cuppett cuppett left a comment

Choose a reason for hiding this comment

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

Unclear if I need to keep this in here after #60002.

Comment thread lib/private/Encryption/EncryptionWrapper.php Outdated
Comment thread lib/private/Encryption/EncryptionWrapper.php Outdated
Comment thread lib/private/Encryption/EncryptionWrapper.php Outdated
Comment thread lib/private/Encryption/EncryptionWrapper.php Outdated
@cuppett cuppett force-pushed the refactor/encryption-wrapper-homemount-support branch from 8ce989d to 74f93c6 Compare May 1, 2026 10:39
@cuppett cuppett requested review from nfebe, sorbaugh and susnux and removed request for a team May 1, 2026 10:39
@cuppett cuppett force-pushed the refactor/encryption-wrapper-homemount-support branch 2 times, most recently from d373b56 to c3abeeb Compare May 1, 2026 11:24
@cuppett
Copy link
Copy Markdown
Contributor Author

cuppett commented May 1, 2026

The psalm things (DAV) seem to be coming from the master branch (not related to this PR). Do I need to fix that or can it be overridden to merge?

@cuppett cuppett requested a review from artonge May 1, 2026 13:51
Copy link
Copy Markdown
Collaborator

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Looks good :)

@cuppett cuppett force-pushed the refactor/encryption-wrapper-homemount-support branch 2 times, most recently from 50d689a to af27b5d Compare May 5, 2026 17:03
@cuppett cuppett enabled auto-merge May 5, 2026 17:04
@cuppett cuppett force-pushed the refactor/encryption-wrapper-homemount-support branch 3 times, most recently from 35e1d9c to 442ddc3 Compare May 6, 2026 15:38
cuppett and others added 3 commits May 6, 2026 23:05
Rewrite conditional flow to use early-return guards: skip IDisableEncryptionStorage,
skip the root mount, respect encryptHomeStorage for HomeMountPoints. Uses IAppConfig
for the encryptHomeStorage setting with a legacy string fallback for the upgrade window.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Stephen Cuppett <steve@cuppett.com>
Signed-off-by: Stephen Cuppett <steve@cuppett.com>
…tion creation

Injecting IAppConfig as a constructor parameter into Encryption\Manager (and
through it into EncryptionWrapper) caused IDBConnection to be eagerly resolved
during OC::init() on PHP <8.4 (no lazy ghost objects). This happened before
maintenance:install's Sqlite::initialize() wrote dbname to config.php, so the
connection latched onto the default database name ('owncloud') instead of the
configured one ('nextcloud'). All migrations then ran against owncloud.db, and
the subsequent enable_all.php process opened an empty nextcloud.db — crashing
with "no such table: oc_appconfig".

Remove IAppConfig from Manager's constructor and Server.php's factory closure.
Resolve it lazily via Server::get(IAppConfig::class) inside
EncryptionWrapper::wrapStorage(), which is only called after the filesystem is
set up, never during bootstrap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Stephen Cuppett <steve@cuppett.com>
@miaulalala miaulalala force-pushed the refactor/encryption-wrapper-homemount-support branch from 442ddc3 to af2453e Compare May 6, 2026 21:05
Comment on lines +75 to +78
if ($mount instanceof HomeMountPoint
&& !Server::get(IAppConfig::class)->getValueBool('encryption', 'encryptHomeStorage', true)) {
return $storage;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This means the encryption wrapper will not be around home storages anymore.

At the very least it will be an issue when the option was set to true and is then set to false, because existing encrypted files will not be decrypted.

Can you detail what this is attempting to fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews feature: encryption (server-side) ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants