E2EE Desync folders on restart of application, computer or logout and back in to user account by Caffe1neAdd1ct · Pull Request #9653 · nextcloud/desktop

Caffe1neAdd1ct · 2026-03-19T23:18:03Z

Continuation from #9379
Bug report: #9345

I feel quite strongly about this issue as it's directly affecting the ability to keep files synced to a device and introducing risks of data loss.

So on this front, please feel free to contribute to this PR and help to progress it.

Two user acceptance tests ran:

https://github.com/user-attachments/assets/82766180-edc3-4aca-a9bc-947365474381 - Before PR (master branch)
https://github.com/user-attachments/assets/23475baf-4386-41eb-877e-d00459fa520c - After PR (this branch at HEAD today)

I've run some other tests including selectively keeping or unchecking multiple e2ee folders and "normal" folders.

This PR does resolve a (in my opinion) serious issue and completely prevents using the e2ee feature. I've been using the original PR branch for a good few weeks now and i've not found any issues with it.

Testing

Sandboxing testing command used:

XDG_CONFIG_HOME=/home/USER_HERE/nextcloud-sandbox/config XDG_DATA_HOME=/home/USER_HERE/nextcloud-sandbox/data bin/nextcloud --confdir /home/USER_HERE/nextcloud-sandbox/config --logfile /tmp/nextcloud-test.log --logdebug

Run this from inside the build output folder and modify paths as needed. Just helped me test without affecting the main user sync settings and folders.

To Do

how the initialization state is updated when pkcs11 hardware tokens are in use
These should be moved to a (private) setInitializationState method, e.g.
set/emits in here can be reduced
This method seems to be unused
how the initialization state is updated when pkcs11 hardware tokens are in use?
tests should be moved to testsyncjournaldb.cpp then as they are not specific to FolderMan at all and really only testing SyncJournalDb
tests do not actually verify any specific behaviour from E2EFolderManager, but from the AccountManager and SyncJournalDb classes

Add comprehensive E2E initialization state tracking with enum-based states to prevent race conditions during client startup. This allows better handling of E2E encrypted folders during the initialization phase. Changes: - Add InitializationState enum (NotStarted, Initializing, Initialized, Failed) - Add isInitializing() and initializationState() query methods - Add initializationStateChanged() signal for state monitoring - Emit state changes during initialization lifecycle - Reset state properly in forgetSensitiveData() for re-initialization Files modified: - src/libsync/clientsideencryption.h - src/libsync/clientsideencryption.cpp Signed-off-by: Kevin Andrews <kevin@nforced.uk>

…ing startup Improve E2E encrypted folder discovery logic to avoid premature blacklisting during client startup race conditions. Only blacklist when E2E truly fails, not while initializing. Changes: - Check E2E initialization state before blacklisting folders - Allow E2E folders to proceed when state is NotStarted or Initializing - Only blacklist when E2E state is Failed or unavailable - Track only NEW blacklisted folders for restoration - Preserve user-chosen blacklisted folders (don't add to restoration list) Files modified: - src/libsync/discovery.cpp Signed-off-by: Kevin Andrews <kevin@nforced.uk>

Fix issue where E2E folders would be incorrectly restored after encryption reset. Clear the restoration tracking list and reset initialization flags when user resets E2E encryption. Changes: - Clear SelectiveSyncE2eFoldersToRemoveFromBlacklist in forgetE2eEncryption() - Reset _e2eAskUserForMnemonic flag to allow re-initialization - Trigger E2E folder restoration after manual E2E setup in setupE2eEncryption()" Signed-off-by: Kevin Andrews <kevin@nforced.uk>

…ialized Fix E2E initialization state not properly transitioning to Initialized state during normal startup when keys are loaded from keychain. This caused folders to remain permanently deferred even though E2E was fully functional. The `privateKeyOnServerIsValid` callback path (used when loading existing keys from keychain) was emitting the `initializationFinished` signal but not setting the internal state to Initialized. Only the `sendPublicKey` path (for new key generation) was correctly updating the state. Changes: - Set `_initializationState = InitializationState::Initialized` in privateKeyOnServerIsValid callback - Emit `initializationStateChanged` signal with updated state - Add debug logging to track state transitions during keychain loading Files modified: - src/libsync/clientsideencryption.cpp Signed-off-by: Kevin Andrews <kevin@nforced.uk>

…nguish from permanent blacklisting Improve E2E encrypted folder discovery logic to properly defer folders during initialization while distinguishing temporary deferral from permanent blacklisting. During client startup, E2E folders are now: - Temporarily deferred when E2E state is NotStarted or Initializing (tracked for restoration) - Permanently blacklisted when E2E state is Failed or capability unavailable (not tracked) This prevents folders with failed E2E from being indefinitely tracked for restoration while still allowing proper restoration when E2E successfully initializes. Changes: - Add `shouldTrackForRestoration` parameter to distinguish temporary vs permanent blacklisting - Defer folders with debug logging when E2E is initializing (tracked for restoration) - Blacklist folders without tracking when E2E failed or unavailable - Preserve user-chosen blacklisted folders (don't override user preferences) Files modified: - src/libsync/discovery.h - src/libsync/discovery.cpp Signed-off-by: Kevin Andrews <kevin@nforced.uk>

…ration Implement E2EFolderManager as a stateless bridge coordinator between E2E encryption and folder management to reliably restore blacklisted E2E folders when encryption becomes available. E2EFolderManager provides: - Initialization at application startup (before first sync) - Connection to all account E2E initialization signals - Automatic restoration of deferred E2E folders when encryption ready - Separation of concerns between encryption and folder management Changes: - Add E2EFolderManager singleton class - Implement `initialize()` to connect to existing accounts and listen for new accounts - Implement `slotE2eInitializationFinished()` to handle E2E ready events - Implement `restoreE2eFoldersForAccount()` to remove folders from blacklist and schedule sync - Initialize E2EFolderManager in Application after FolderMan setup - Add comprehensive debug logging with lcE2eFolderManager category Files added: - src/gui/e2efoldermanager.h - src/gui/e2efoldermanager.cpp Files modified: - src/gui/CMakeLists.txt - src/gui/application.cpp - src/gui/accountsettings.cpp Signed-off-by: Kevin Andrews <kevin@nforced.uk>

… restoration logic Signed-off-by: Kevin Andrews <kevin@nforced.uk>

Signed-off-by: Kevin Andrews <kevin@nforced.uk>

…apability early returns Signed-off-by: Kevin Andrews <kevin@nforced.uk>

Signed-off-by: Kevin Andrews <kevin@nforced.uk>

nilsding

Thanks again for your contribution, the overall implementation looks fine to me :)

I am mainly concerned about the test cases -- I think most of them are good to have (might be worth to introduce new test files in general, e.g. for AccountManager), however to me they do not seem to really test the behaviour of the new E2eFolderManager class.

src/libsync/clientsideencryption.cpp

nilsding · 2026-03-20T08:55:51Z

test/testfolderman.cpp

+    void testE2EFolderBlacklistRestoration()
+    {
+        // Test that E2E folders can be tracked in the database for restoration
+        // This test verifies the database operations work without requiring full FolderMan setup


These tests should be moved to testsyncjournaldb.cpp then as they are not specific to FolderMan at all and really only testing SyncJournalDb

Tests relocated from testfolderman.cpp :

testE2EFolderBlacklistRestoration
testE2EFolderNotTrackedIfUserBlacklisted
testE2ERestorationClearsTrackingList
have all been completely removed .

to testsyncjournaldb.cpp for testing SyncJournalDb schema without FolderMan mockups

@nilsding can you review this please?

nilsding · 2026-03-20T10:03:00Z

test/teste2eefoldermanager.cpp

+        account->setCapabilities(capabilities);
+
+        auto accountState = new AccountState(account);
+        accountState->setParent(this);
+        AccountManager::instance()->addAccount(account);
+
+        // Simulate folders blacklisted during startup (before E2E initialized)
+        // This mimics what happens when client restarts and E2E isn't ready yet
+        QTemporaryDir syncDir;
+        QString dbPath = syncDir.path() + "/.sync_test.db";
+        SyncJournalDb db(dbPath);
+
+        QStringList e2eFoldersToRestore = {"/encrypted1/", "/encrypted2/"};
+        db.setSelectiveSyncList(
+            SyncJournalDb::SelectiveSyncE2eFoldersToRemoveFromBlacklist,
+            e2eFoldersToRestore);
+
+        // Verify folders are marked for restoration
+        bool ok = false;
+        auto restorationList = db.getSelectiveSyncList(
+            SyncJournalDb::SelectiveSyncE2eFoldersToRemoveFromBlacklist, &ok);
+        QVERIFY(ok);
+        QCOMPARE(restorationList.size(), 2);
+
+        // E2EFolderManager is ready to restore folders when E2E signal fires
+        auto *manager = E2EFolderManager::instance();
+        QVERIFY(manager);
+        QVERIFY(account->e2e());
+    }
+
+    void testScenario5_MultipleFoldersTrackedForRestoration()
+    {
+        // Verify multiple E2E folders can be tracked and restored
+        QTemporaryDir dir;
+        QString dbPath = dir.path() + "/.sync_test.db";
+        SyncJournalDb db(dbPath);
+
+        // Simulate multiple E2E folders being blacklisted during startup
+        QStringList multipleFolders = {
+            "/Documents/Private/",
+            "/Photos/Encrypted/",
+            "/Work/Confidential/",
+            "/Personal/Secrets/"
+        };
+
+        db.setSelectiveSyncList(
+            SyncJournalDb::SelectiveSyncE2eFoldersToRemoveFromBlacklist,
+            multipleFolders);
+
+        // Verify all folders are tracked
+        bool ok = false;
+        auto restorationList = db.getSelectiveSyncList(
+            SyncJournalDb::SelectiveSyncE2eFoldersToRemoveFromBlacklist, &ok);
+        QVERIFY(ok);
+        QCOMPARE(restorationList.size(), 4);
+
+        for (const auto &folder : multipleFolders) {
+            QVERIFY(restorationList.contains(folder));
+        }
+
+        // After restoration, list should be clearable
+        db.setSelectiveSyncList(
+            SyncJournalDb::SelectiveSyncE2eFoldersToRemoveFromBlacklist,
+            {});
+
+        restorationList = db.getSelectiveSyncList(
+            SyncJournalDb::SelectiveSyncE2eFoldersToRemoveFromBlacklist, &ok);
+        QVERIFY(ok);
+        QVERIFY(restorationList.isEmpty());
+    }
+
+    void testScenario6_UserBlacklistPreserved()
+    {
+        // Verify user-blacklisted folders are NOT added to restoration list
+        QTemporaryDir dir;
+        QString dbPath = dir.path() + "/.sync_test.db";
+        SyncJournalDb db(dbPath);
+
+        // User manually blacklists an E2E folder via selective sync
+        QStringList userBlacklist = {"/User/Excluded/"};
+        db.setSelectiveSyncList(
+            SyncJournalDb::SelectiveSyncBlackList,
+            userBlacklist);
+
+        // Verify it's blacklisted
+        bool ok = false;
+        auto blacklist = db.getSelectiveSyncList(
+            SyncJournalDb::SelectiveSyncBlackList, &ok);
+        QVERIFY(ok);
+        QCOMPARE(blacklist.size(), 1);
+        QVERIFY(blacklist.contains("/User/Excluded/"));
+
+        // Verify it's NOT in restoration list (would be added only during E2E init)
+        auto restorationList = db.getSelectiveSyncList(
+            SyncJournalDb::SelectiveSyncE2eFoldersToRemoveFromBlacklist, &ok);
+        QVERIFY(ok);
+        QVERIFY(restorationList.isEmpty());
+
+        // This ensures user preferences are preserved across restarts
+    }


just as in the previous PR #9379, most of these tests do not actually verify any specific behaviour from E2EFolderManager, but from the AccountManager and SyncJournalDb classes. The tests in here that make use of SyncJournalDb are directly modifying a sync state database which are not part of any folder connection.

You would need to try to properly set up an account with a sync connection (maybe using the FakeFolder test helper class could be a starting point for that -- this one sets up a fake folder for testing complete with a folder-specific sync journal but does not register it with FolderMan, see https://github.com/nextcloud/desktop/blob/master/test/testfolderman.cpp?plain=1#L123-L126 for an example on how this would be achieved), update the capabilities of the account to indicate support for end-to-end-encryption, and e.g. verify the folder's journal db before and after adding the account to AccountManager.
The current state of the testScenario1_FoldersRestoreAfterRestart test is looking really promising, but it should be creating and updating a sync journal DB by itself (other than preparing it to a state where you can verify the behaviour of E2EFolderManager afterwards of course).

I've had a go at improving the e2ee folder manager tests:

removed QTemporaryDir and SyncJournalDb instantiations

creating a FakeFolder folder object

creating an account with the folder and setup e2ee capabilities for it

query the fake folder sync journals rather than seeding and relying on a db instance

Let me know your thoughts @nilsding

Caffe1neAdd1ct · 2026-03-20T10:26:16Z

Thanks again for your contribution, the overall implementation looks fine to me :)

I am mainly concerned about the test cases -- I think most of them are good to have (might be worth to introduce new test files in general, e.g. for AccountManager), however to me they do not seem to really test the behaviour of the new E2eFolderManager class.

Thanks @nilsding I'll have a look at the tests as soon as possible, life is on the busy front at the moment with a personal P1 deadline looming 👶 and outstanding DIY renovations needing urgent attention 😳

I've improved on the other areas mentioned and cleaned up the implementation quite a bit, tests definitely needing improvements.

Hopefully there will be some time next week or the following week.

Thanks for the pointers 🙏

Signed-off-by: Kevin Andrews <kevin@nforced.uk>

…ve blacklist handling Signed-off-by: Kevin Andrews <kevin@nforced.uk>

…sful initialization using hardware token Signed-off-by: Kevin Andrews <kevin@nforced.uk>

Caffe1neAdd1ct · 2026-03-21T00:11:08Z

Looking into a couple of regressions these changes have introduced:

names of sub-directories are mangled and not replaced with their correct decrypted names
full redownloads of the e2ee folders started on every restart of the client application

had these issues before, race conditions where the e2ee isn't ready but the sync had started

Caffe1neAdd1ct · 2026-03-21T00:21:02Z

Added back in the continue's inside shouldDeferE2eFolder and isEncryptedFolderButE2eIsNotSetup checks

Removed the || isEncryptedFolderButE2eIsNotSetup check

folderTerminateSyncAndUpdateBlackList and if (blackList.size() != blackListSize) { safety checks added back in

Client no longer re-downloads all files each client startup for e2ee folders.

…c termination and blacklist updates Signed-off-by: Kevin Andrews <kevin@nforced.uk>

…ger tests Signed-off-by: Kevin Andrews <kevin@nforced.uk>

Caffe1neAdd1ct added 9 commits January 22, 2026 00:16

test(nextcloudgh-9345): add E2EFolderManager tests and improve folder…

66cca74

… restoration logic Signed-off-by: Kevin Andrews <kevin@nforced.uk>

fix(nextcloudgh-9345): address all review feedback from PR 9379

44623ac

Signed-off-by: Kevin Andrews <kevin@nforced.uk>

fix(nextcloudgh-9345): address PR feedback, clean test logs, handle c…

3ef9a14

…apability early returns Signed-off-by: Kevin Andrews <kevin@nforced.uk>

Caffe1neAdd1ct requested review from Aiiaiiio, camilasan, i2h3, mgallien and nilsding as code owners March 19, 2026 23:18

Caffe1neAdd1ct added 2 commits March 19, 2026 23:24

fix(nextcloudgh-9345): clean up comments

89e89d4

Signed-off-by: Kevin Andrews <kevin@nforced.uk>

fix(nextcloudgh-9345): remove maybe_unused and improved qverify asserts

9067ac9

Signed-off-by: Kevin Andrews <kevin@nforced.uk>

nilsding requested changes Mar 20, 2026

View reviewed changes

Caffe1neAdd1ct force-pushed the bugfix/gh-9345-e2e-accountsettings branch 2 times, most recently from 16595d6 to 689fcbf Compare March 20, 2026 22:46

Caffe1neAdd1ct added 3 commits March 20, 2026 22:48

refactor(nextcloudgh-9345): consolidate initialization state management

743aeb4

Signed-off-by: Kevin Andrews <kevin@nforced.uk>

fix(nextcloudgh-9345): enhance E2E folder restoration tests and impro…

6186298

…ve blacklist handling Signed-off-by: Kevin Andrews <kevin@nforced.uk>

fix(nextcloudgh-9345): set initialization state to failed on unsucces…

4321f21

…sful initialization using hardware token Signed-off-by: Kevin Andrews <kevin@nforced.uk>

Caffe1neAdd1ct force-pushed the bugfix/gh-9345-e2e-accountsettings branch from 689fcbf to 4321f21 Compare March 20, 2026 22:49

Caffe1neAdd1ct requested a review from nilsding March 23, 2026 22:33

Caffe1neAdd1ct added 2 commits March 23, 2026 22:34

fix(nextcloudgh-9345): improve E2E folder restoration by handling syn…

ed5abe1

…c termination and blacklist updates Signed-off-by: Kevin Andrews <kevin@nforced.uk>

fix(nextcloudgh-9345): add initialization logging for E2E folder mana…

b554564

…ger tests Signed-off-by: Kevin Andrews <kevin@nforced.uk>

Caffe1neAdd1ct force-pushed the bugfix/gh-9345-e2e-accountsettings branch from be25c2b to b554564 Compare March 23, 2026 22:34

Merge branch 'master' into bugfix/nextcloudgh-9345-e2e-accountsettings

2c96643

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

E2EE Desync folders on restart of application, computer or logout and back in to user account#9653

E2EE Desync folders on restart of application, computer or logout and back in to user account#9653
Caffe1neAdd1ct wants to merge 17 commits intonextcloud:masterfrom
Caffe1neAdd1ct:bugfix/gh-9345-e2e-accountsettings

Caffe1neAdd1ct commented Mar 19, 2026 •

edited

Loading

Uh oh!

nilsding left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nilsding Mar 20, 2026

Uh oh!

Caffe1neAdd1ct Mar 20, 2026

Uh oh!

nilsding Mar 20, 2026

Uh oh!

Caffe1neAdd1ct Mar 20, 2026

Uh oh!

Caffe1neAdd1ct commented Mar 20, 2026

Uh oh!

Caffe1neAdd1ct commented Mar 21, 2026

Uh oh!

Caffe1neAdd1ct commented Mar 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Caffe1neAdd1ct commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nilsding left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nilsding Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Caffe1neAdd1ct Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

nilsding Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Caffe1neAdd1ct Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Caffe1neAdd1ct commented Mar 20, 2026

Uh oh!

Caffe1neAdd1ct commented Mar 21, 2026

Uh oh!

Caffe1neAdd1ct commented Mar 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Caffe1neAdd1ct commented Mar 19, 2026 •

edited

Loading