chore: Umshiang Bridge 3.5.2 changelog.

After we detect that the user has suffered the GODT-3003 bug due the
vault corruption not ensuring that a previous sync state would be
erased, we patch the gluon db directly and then reset the sync state.

After the account is added, the sync is automatically triggered and the
account state fixes itself.

Changelog.md

@@ -3,9 +3,28 @@
 Changelog [format](http://keepachangelog.com/en/1.0.0/)
 
 
+## Umshiang Bridge 3.5.2
+
+### Fixed
+* GODT-3003: Ensure IMAP State is reset after vault corruption.
+* GODT-3001: Only create system labels during system label sync.
+
+
+## Umshiang Bridge 3.5.1
+
+### Fixed
+* GODT-2963: Use multi error to report file removal errors.
+* GODT-2956: Restore old deletion rules.
+* GODT-2951: Negative WaitGroup Counter.
+* GODT-2590: Fix send on closed channel.
+* GODT-2949: Fix close of close channel in event service.
+
+
 ## Umshiang Bridge 3.5.0
 
 ### Added
+* GODT-2734: Add testing steps to modify account settings.
+* GODT-2746: Integration tests for reporting a problem.
 * GODT-2891: Allow message create & delete during sync.
 * GODT-2848: Decouple IMAP service from Event Loop.
 * Add trace profiling option.
@@ -19,6 +38,8 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 * GODT-2803: Bridge Database access.
 
 ### Changed
+* GODT-2909: Remove Timeout on event publish.
+* GODT-2913: Reduce the number of configuration failure detected.
 * GODT-2828: Increase sync progress report frequency.
 * Test: Fix TestBridge_SyncWithOnGoingEvents.
 * GODT-2871: Is telemetry enabled as service.
@@ -71,6 +92,7 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 * GODT-2780: Fix 'QSystemTrayIcon::setVisible: No Icon set' warning in bridge-gui log on startup.
 * GODT-2778: Fix login screen being disabled after an 'already logged in' error.
 * Fix typos found by codespell.
+* GODT-2577: Answered flag should only be applied to replied messages.
 
 
 ## Trift Bridge 3.4.1

Makefile

@@ -11,7 +11,7 @@ ROOT_DIR:=$(shell dirname $(realpath $(firstword $(MAKEFILE_LIST))))
 .PHONY: build build-gui build-nogui build-launcher versioner hasher
 
 # Keep version hardcoded so app build works also without Git repository.
-BRIDGE_APP_VERSION?=3.5.0+git
+BRIDGE_APP_VERSION?=3.5.2+git
 APP_VERSION:=${BRIDGE_APP_VERSION}
 APP_FULL_NAME:=Proton Mail Bridge
 APP_VENDOR:=Proton AG
@@ -304,6 +304,7 @@ ApplyStageInput,BuildStageInput,BuildStageOutput,DownloadStageInput,DownloadStag
 StateProvider,Regulator,UpdateApplier,MessageBuilder,APIClient,Reporter,DownloadRateModifier \
 > tmp
 	mv tmp internal/services/syncservice/mocks_test.go
+	mockgen --package mocks github.com/ProtonMail/gluon/connector IMAPStateWrite > internal/services/imapservice/mocks/mocks.go
 
 lint: gofiles lint-golang lint-license lint-dependencies lint-changelog lint-bug-report
 

go.mod

@@ -5,7 +5,7 @@ go 1.20
 require (
 	github.com/0xAX/notificator v0.0.0-20220220101646-ee9b8921e557
 	github.com/Masterminds/semver/v3 v3.2.0
-	github.com/ProtonMail/gluon v0.17.1-0.20230829112217-5d5c25c504b5
+	github.com/ProtonMail/gluon v0.17.1-0.20231009084701-3af0474b0b3c
 	github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a
 	github.com/ProtonMail/go-proton-api v0.4.1-0.20230831064234-0e3a549b3f36
 	github.com/ProtonMail/gopenpgp/v2 v2.7.1-proton

go.sum

@@ -23,8 +23,8 @@ github.com/ProtonMail/bcrypt v0.0.0-20211005172633-e235017c1baf h1:yc9daCCYUefEs
 github.com/ProtonMail/bcrypt v0.0.0-20211005172633-e235017c1baf/go.mod h1:o0ESU9p83twszAU8LBeJKFAAMX14tISa0yk4Oo5TOqo=
 github.com/ProtonMail/docker-credential-helpers v1.1.0 h1:+kvUIpwWcbtP3WFv5sSvkFn/XLzSqPOB5AAthuk9xPk=
 github.com/ProtonMail/docker-credential-helpers v1.1.0/go.mod h1:mK0aBveCxhnQ756AmaTfXMZDeULvheYVhF/MWMErN5g=
-github.com/ProtonMail/gluon v0.17.1-0.20230829112217-5d5c25c504b5 h1:C/8P5NHAKi2yCKez+OZ5rSR8SsL7k8si4pK4SE2QtV8=
-github.com/ProtonMail/gluon v0.17.1-0.20230829112217-5d5c25c504b5/go.mod h1:Og5/Dz1MiGpCJn51XujZwxiLG7WzvvjE5PRpZBQmAHo=
+github.com/ProtonMail/gluon v0.17.1-0.20231009084701-3af0474b0b3c h1:gUDu4pOswgbou0QczfreNiXQFrmvVlpSh8Q+vft/JvI=
+github.com/ProtonMail/gluon v0.17.1-0.20231009084701-3af0474b0b3c/go.mod h1:Og5/Dz1MiGpCJn51XujZwxiLG7WzvvjE5PRpZBQmAHo=
 github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a h1:D+aZah+k14Gn6kmL7eKxoo/4Dr/lK3ChBcwce2+SQP4=
 github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a/go.mod h1:oTGdE7/DlWIr23G0IKW3OXK9wZ5Hw1GGiaJFccTvZi4=
 github.com/ProtonMail/go-crypto v0.0.0-20230321155629-9a39f2531310/go.mod h1:8TI4H3IbrackdNgv+92dI+rhpCaLqM0IfpgCgenFvRE=

internal/bridge/bridge_test.go

@@ -585,7 +585,7 @@ func TestBridge_MissingGluonStore(t *testing.T) {
 		require.NoError(t, os.RemoveAll(gluonDir))
 
 		// Bridge starts but can't find the gluon store dir; there should be no error.
-		withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
+		withBridgeWaitForServers(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
 			// ...
 		})
 	})

internal/bridge/sync_test.go

@@ -37,6 +37,7 @@ import (
 	"github.com/ProtonMail/proton-bridge/v3/internal/bridge"
 	"github.com/ProtonMail/proton-bridge/v3/internal/constants"
 	"github.com/ProtonMail/proton-bridge/v3/internal/events"
+	"github.com/ProtonMail/proton-bridge/v3/internal/services/imapservice"
 	"github.com/bradenaw/juniper/iterator"
 	"github.com/bradenaw/juniper/stream"
 	"github.com/bradenaw/juniper/xslices"
@@ -579,6 +580,67 @@ func TestBridge_MessageCreateDuringSync(t *testing.T) {
 	}, server.WithTLS(false))
 }
 
+func TestBridge_CorruptedVaultClearsPreviousIMAPSyncState(t *testing.T) {
+	withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, vaultKey []byte) {
+		userID, addrID, err := s.CreateUser("imap", password)
+		require.NoError(t, err)
+
+		labelID, err := s.CreateLabel(userID, "folder", "", proton.LabelTypeFolder)
+		require.NoError(t, err)
+
+		withClient(ctx, t, s, "imap", password, func(ctx context.Context, c *proton.Client) {
+			createNumMessages(ctx, t, c, addrID, labelID, 100)
+		})
+
+		withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
+			syncCh, done := chToType[events.Event, events.SyncFinished](bridge.GetEvents(events.SyncFinished{}))
+			defer done()
+
+			var err error
+
+			userID, err = bridge.LoginFull(context.Background(), "imap", password, nil, nil)
+			require.NoError(t, err)
+
+			// Wait for sync to finish
+			require.Equal(t, userID, (<-syncCh).UserID)
+		})
+
+		settingsPath, err := locator.ProvideSettingsPath()
+		require.NoError(t, err)
+
+		syncConfigPath, err := locator.ProvideIMAPSyncConfigPath()
+		require.NoError(t, err)
+
+		syncStatePath := imapservice.GetSyncConfigPath(syncConfigPath, userID)
+		// Check sync state is complete
+		{
+			state, err := imapservice.NewSyncState(syncStatePath)
+			require.NoError(t, err)
+			syncStatus, err := state.GetSyncStatus(context.Background())
+			require.NoError(t, err)
+			require.True(t, syncStatus.IsComplete())
+		}
+
+		// corrupt the vault
+		require.NoError(t, os.WriteFile(filepath.Join(settingsPath, "vault.enc"), []byte("Trash!"), 0o600))
+
+		// Bridge starts but can't find the gluon database dir; there should be no error.
+		withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
+			_, err := bridge.LoginFull(context.Background(), "imap", password, nil, nil)
+			require.NoError(t, err)
+		})
+
+		// Check sync state is reset.
+		{
+			state, err := imapservice.NewSyncState(syncStatePath)
+			require.NoError(t, err)
+			syncStatus, err := state.GetSyncStatus(context.Background())
+			require.NoError(t, err)
+			require.False(t, syncStatus.IsComplete())
+		}
+	})
+}
+
 func withClient(ctx context.Context, t *testing.T, s *server.Server, username string, password []byte, fn func(context.Context, *proton.Client)) { //nolint:unparam
 	m := proton.New(
 		proton.WithHostURL(s.GetHostURL()),

internal/services/imapservice/connector.go

@@ -37,6 +37,7 @@ import (
 	"github.com/ProtonMail/proton-bridge/v3/pkg/message"
 	"github.com/ProtonMail/proton-bridge/v3/pkg/message/parser"
 	"github.com/bradenaw/juniper/stream"
+	"github.com/bradenaw/juniper/xslices"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/exp/slices"
 )
@@ -62,6 +63,7 @@ type Connector struct {
 	log         *logrus.Entry
 
 	sharedCache *SharedCache
+	syncState   *SyncState
 }
 
 func NewConnector(
@@ -74,6 +76,7 @@ func NewConnector(
 	panicHandler async.PanicHandler,
 	telemetry Telemetry,
 	showAllMail bool,
+	syncState *SyncState,
 ) *Connector {
 	userID := identityState.UserID()
 
@@ -105,6 +108,7 @@ func NewConnector(
 		}),
 
 		sharedCache: NewSharedCached(),
+		syncState:   syncState,
 	}
 }
 
@@ -113,9 +117,35 @@ func (s *Connector) StateClose() {
 	s.updateCh.CloseAndDiscardQueued()
 }
 
-func (s *Connector) Init(_ context.Context, cache connector.IMAPState) error {
+func (s *Connector) Init(ctx context.Context, cache connector.IMAPState) error {
 	s.sharedCache.Set(cache)
+
+	return cache.Write(ctx, func(ctx context.Context, write connector.IMAPStateWrite) error {
+		rd := s.labels.Read()
+		defer rd.Close()
+
+		mboxes, err := write.GetMailboxesWithoutAttrib(ctx)
+		if err != nil {
+			return err
+		}
+
+		// Attempt to fix bug when a vault got corrupted, but the sync state did not get reset leading to
+		// all labels being written to the root level. If we detect this happened, reset the sync state.
+		{
+			applied, err := fixGODT3003Labels(ctx, s.log, mboxes, rd, write)
+			if err != nil {
+				return err
+			}
+
+			if applied {
+				s.log.Debug("Patched folders/labels after GODT-3003 incident, resetting sync state.")
+				if err := s.syncState.ClearSyncStatus(ctx); err != nil {
+					return err
+				}
+			}
+		}
 		return nil
+	})
 }
 
 func (s *Connector) Authorize(ctx context.Context, username string, password []byte) bool {
@@ -334,9 +364,70 @@ func (s *Connector) RemoveMessagesFromMailbox(ctx context.Context, _ connector.I
 	}
 
 	if mboxID == proton.TrashLabel || mboxID == proton.DraftsLabel {
-		if err := s.client.DeleteMessage(ctx, msgIDs...); err != nil {
+		const ChunkSize = 150
+		var msgToPermaDelete []string
+
+		rdLabels := s.labels.Read()
+		defer rdLabels.Close()
+
+		// There's currently no limit on how many IDs we can filter on,
+		// but to be nice to API, let's chunk it by 150.
+		for _, messageIDs := range xslices.Chunk(messageIDs, ChunkSize) {
+			metadata, err := s.client.GetMessageMetadataPage(ctx, 0, ChunkSize, proton.MessageFilter{
+				ID: usertypes.MapTo[imap.MessageID, string](messageIDs),
+			})
+			if err != nil {
 				return err
 			}
+
+			// If a message is not preset in any other label other than AllMail, AllDrafts and AllSent, it can be
+			// permanently deleted.
+			for _, m := range metadata {
+				var remainingLabels []string
+
+				for _, id := range m.LabelIDs {
+					label, ok := rdLabels.GetLabel(id)
+					if !ok {
+						// Handle case where this label was newly introduced and we do not yet know about it.
+						logrus.WithField("labelID", id).Warnf("Unknown label found during expung from Trash, attempting to locate it")
+						label, err = s.client.GetLabel(ctx, id, proton.LabelTypeFolder, proton.LabelTypeSystem, proton.LabelTypeSystem)
+						if err != nil {
+							if errors.Is(err, proton.ErrNoSuchLabel) {
+								logrus.WithField("labelID", id).Warn("Label does not exist, ignoring")
+								continue
+							}
+
+							logrus.WithField("labelID", id).Errorf("Failed to resolve label: %v", err)
+							return fmt.Errorf("failed to resolve label: %w", err)
+						}
+					}
+					if !WantLabel(label) {
+						continue
+					}
+
+					if label.Type == proton.LabelTypeSystem && (id == proton.AllDraftsLabel ||
+						id == proton.AllMailLabel ||
+						id == proton.AllSentLabel ||
+						id == proton.AllScheduledLabel) {
+						continue
+					}
+
+					remainingLabels = append(remainingLabels, m.ID)
+				}
+
+				if len(remainingLabels) == 0 {
+					msgToPermaDelete = append(msgToPermaDelete, m.ID)
+				}
+			}
+		}
+
+		if len(msgToPermaDelete) != 0 {
+			logrus.Debugf("Following message(s) will be perma-deleted: %v", msgToPermaDelete)
+
+			if err := s.client.DeleteMessage(ctx, msgToPermaDelete...); err != nil {
+				return err
+			}
+		}
 	}
 
 	return nil
@@ -683,3 +774,41 @@ func (s *Connector) createDraft(ctx context.Context, literal []byte, addrKR *cry
 func (s *Connector) publishUpdate(_ context.Context, update imap.Update) {
 	s.updateCh.Enqueue(update)
 }
+
+func fixGODT3003Labels(
+	ctx context.Context,
+	log *logrus.Entry,
+	mboxes []imap.MailboxNoAttrib,
+	rd labelsRead,
+	write connector.IMAPStateWrite,
+) (bool, error) {
+	var applied bool
+	for _, mbox := range mboxes {
+		lbl, ok := rd.GetLabel(string(mbox.ID))
+		if !ok {
+			continue
+		}
+
+		if lbl.Type == proton.LabelTypeFolder {
+			if mbox.Name[0] != folderPrefix {
+				log.WithField("labelID", mbox.ID.ShortID()).Debug("Found folder without prefix, patching")
+				if err := write.PatchMailboxHierarchyWithoutTransforms(ctx, mbox.ID, xslices.Insert(mbox.Name, 0, folderPrefix)); err != nil {
+					return false, fmt.Errorf("failed to update mailbox name: %w", err)
+				}
+
+				applied = true
+			}
+		} else if lbl.Type == proton.LabelTypeLabel {
+			if mbox.Name[0] != labelPrefix {
+				log.WithField("labelID", mbox.ID.ShortID()).Debug("Found label without prefix, patching")
+				if err := write.PatchMailboxHierarchyWithoutTransforms(ctx, mbox.ID, xslices.Insert(mbox.Name, 0, labelPrefix)); err != nil {
+					return false, fmt.Errorf("failed to update mailbox name: %w", err)
+				}
+
+				applied = true
+			}
+		}
+	}
+
+	return applied, nil
+}

internal/services/imapservice/connector_test.go

@@ -0,0 +1,205 @@
+// Copyright (c) 2023 Proton AG
+//
+// This file is part of Proton Mail Bridge.
+//
+// Proton Mail Bridge is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Proton Mail Bridge is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Proton Mail Bridge.  If not, see <https://www.gnu.org/licenses/>.
+
+package imapservice
+
+import (
+	"context"
+	"testing"
+
+	"github.com/ProtonMail/gluon/imap"
+	"github.com/ProtonMail/go-proton-api"
+	"github.com/ProtonMail/proton-bridge/v3/internal/services/imapservice/mocks"
+	"github.com/golang/mock/gomock"
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFixGODT3003Labels(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+
+	log := logrus.WithField("test", "test")
+
+	sharedLabels := newRWLabels()
+	wr := sharedLabels.Write()
+	wr.SetLabel("foo", proton.Label{
+		ID:       "foo",
+		ParentID: "bar",
+		Name:     "Foo",
+		Path:     []string{"bar", "Foo"},
+		Color:    "",
+		Type:     proton.LabelTypeFolder,
+	})
+
+	wr.SetLabel("0", proton.Label{
+		ID:       "0",
+		ParentID: "",
+		Name:     "Inbox",
+		Path:     []string{"Inbox"},
+		Color:    "",
+		Type:     proton.LabelTypeSystem,
+	})
+
+	wr.SetLabel("bar", proton.Label{
+		ID:       "bar",
+		ParentID: "",
+		Name:     "boo",
+		Path:     []string{"bar"},
+		Color:    "",
+		Type:     proton.LabelTypeFolder,
+	})
+
+	wr.SetLabel("my_label", proton.Label{
+		ID:       "my_label",
+		ParentID: "",
+		Name:     "MyLabel",
+		Path:     []string{"MyLabel"},
+		Color:    "",
+		Type:     proton.LabelTypeLabel,
+	})
+
+	wr.SetLabel("my_label2", proton.Label{
+		ID:       "my_label2",
+		ParentID: "",
+		Name:     "MyLabel2",
+		Path:     []string{labelPrefix, "MyLabel2"},
+		Color:    "",
+		Type:     proton.LabelTypeLabel,
+	})
+	wr.Close()
+
+	mboxs := []imap.MailboxNoAttrib{
+		{
+			ID:   "0",
+			Name: []string{"Inbox"},
+		},
+		{
+			ID:   "bar",
+			Name: []string{"bar"},
+		},
+		{
+			ID:   "foo",
+			Name: []string{"bar", "Foo"},
+		},
+		{
+			ID:   "my_label",
+			Name: []string{"MyLabel"},
+		},
+		{
+			ID:   "my_label2",
+			Name: []string{labelPrefix, "MyLabel2"},
+		},
+	}
+
+	rd := sharedLabels.Read()
+	defer rd.Close()
+
+	imapState := mocks.NewMockIMAPStateWrite(mockCtrl)
+
+	imapState.EXPECT().PatchMailboxHierarchyWithoutTransforms(gomock.Any(), gomock.Eq(imap.MailboxID("bar")), gomock.Eq([]string{folderPrefix, "bar"}))
+	imapState.EXPECT().PatchMailboxHierarchyWithoutTransforms(gomock.Any(), gomock.Eq(imap.MailboxID("foo")), gomock.Eq([]string{folderPrefix, "bar", "Foo"}))
+	imapState.EXPECT().PatchMailboxHierarchyWithoutTransforms(gomock.Any(), gomock.Eq(imap.MailboxID("my_label")), gomock.Eq([]string{labelPrefix, "MyLabel"}))
+
+	applied, err := fixGODT3003Labels(context.Background(), log, mboxs, rd, imapState)
+	require.NoError(t, err)
+	require.True(t, applied)
+}
+
+func TestFixGODT3003Labels_Noop(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+
+	log := logrus.WithField("test", "test")
+
+	sharedLabels := newRWLabels()
+	wr := sharedLabels.Write()
+	wr.SetLabel("foo", proton.Label{
+		ID:       "foo",
+		ParentID: "bar",
+		Name:     "Foo",
+		Path:     []string{folderPrefix, "bar", "Foo"},
+		Color:    "",
+		Type:     proton.LabelTypeFolder,
+	})
+
+	wr.SetLabel("0", proton.Label{
+		ID:       "0",
+		ParentID: "",
+		Name:     "Inbox",
+		Path:     []string{"Inbox"},
+		Color:    "",
+		Type:     proton.LabelTypeSystem,
+	})
+
+	wr.SetLabel("bar", proton.Label{
+		ID:       "bar",
+		ParentID: "",
+		Name:     "bar",
+		Path:     []string{folderPrefix, "bar"},
+		Color:    "",
+		Type:     proton.LabelTypeFolder,
+	})
+
+	wr.SetLabel("my_label", proton.Label{
+		ID:       "my_label",
+		ParentID: "",
+		Name:     "MyLabel",
+		Path:     []string{labelPrefix, "MyLabel"},
+		Color:    "",
+		Type:     proton.LabelTypeLabel,
+	})
+
+	wr.SetLabel("my_label2", proton.Label{
+		ID:       "my_label2",
+		ParentID: "",
+		Name:     "MyLabel2",
+		Path:     []string{labelPrefix, "MyLabel2"},
+		Color:    "",
+		Type:     proton.LabelTypeLabel,
+	})
+	wr.Close()
+
+	mboxs := []imap.MailboxNoAttrib{
+		{
+			ID:   "0",
+			Name: []string{"Inbox"},
+		},
+		{
+			ID:   "bar",
+			Name: []string{folderPrefix, "bar"},
+		},
+		{
+			ID:   "foo",
+			Name: []string{folderPrefix, "bar", "Foo"},
+		},
+		{
+			ID:   "my_label",
+			Name: []string{labelPrefix, "MyLabel"},
+		},
+		{
+			ID:   "my_label2",
+			Name: []string{labelPrefix, "MyLabel2"},
+		},
+	}
+
+	rd := sharedLabels.Read()
+	defer rd.Close()
+
+	imapState := mocks.NewMockIMAPStateWrite(mockCtrl)
+	applied, err := fixGODT3003Labels(context.Background(), log, mboxs, rd, imapState)
+	require.NoError(t, err)
+	require.False(t, applied)
+}

internal/services/imapservice/mocks/mocks.go

@@ -0,0 +1,138 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: github.com/ProtonMail/gluon/connector (interfaces: IMAPStateWrite)
+
+// Package mocks is a generated GoMock package.
+package mocks
+
+import (
+	context "context"
+	reflect "reflect"
+
+	imap "github.com/ProtonMail/gluon/imap"
+	gomock "github.com/golang/mock/gomock"
+)
+
+// MockIMAPStateWrite is a mock of IMAPStateWrite interface.
+type MockIMAPStateWrite struct {
+	ctrl     *gomock.Controller
+	recorder *MockIMAPStateWriteMockRecorder
+}
+
+// MockIMAPStateWriteMockRecorder is the mock recorder for MockIMAPStateWrite.
+type MockIMAPStateWriteMockRecorder struct {
+	mock *MockIMAPStateWrite
+}
+
+// NewMockIMAPStateWrite creates a new mock instance.
+func NewMockIMAPStateWrite(ctrl *gomock.Controller) *MockIMAPStateWrite {
+	mock := &MockIMAPStateWrite{ctrl: ctrl}
+	mock.recorder = &MockIMAPStateWriteMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockIMAPStateWrite) EXPECT() *MockIMAPStateWriteMockRecorder {
+	return m.recorder
+}
+
+// CreateMailbox mocks base method.
+func (m *MockIMAPStateWrite) CreateMailbox(arg0 context.Context, arg1 imap.Mailbox) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "CreateMailbox", arg0, arg1)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// CreateMailbox indicates an expected call of CreateMailbox.
+func (mr *MockIMAPStateWriteMockRecorder) CreateMailbox(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateMailbox", reflect.TypeOf((*MockIMAPStateWrite)(nil).CreateMailbox), arg0, arg1)
+}
+
+// GetMailboxCount mocks base method.
+func (m *MockIMAPStateWrite) GetMailboxCount(arg0 context.Context) (int, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "GetMailboxCount", arg0)
+	ret0, _ := ret[0].(int)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// GetMailboxCount indicates an expected call of GetMailboxCount.
+func (mr *MockIMAPStateWriteMockRecorder) GetMailboxCount(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMailboxCount", reflect.TypeOf((*MockIMAPStateWrite)(nil).GetMailboxCount), arg0)
+}
+
+// GetMailboxesWithoutAttrib mocks base method.
+func (m *MockIMAPStateWrite) GetMailboxesWithoutAttrib(arg0 context.Context) ([]imap.MailboxNoAttrib, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "GetMailboxesWithoutAttrib", arg0)
+	ret0, _ := ret[0].([]imap.MailboxNoAttrib)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// GetMailboxesWithoutAttrib indicates an expected call of GetMailboxesWithoutAttrib.
+func (mr *MockIMAPStateWriteMockRecorder) GetMailboxesWithoutAttrib(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMailboxesWithoutAttrib", reflect.TypeOf((*MockIMAPStateWrite)(nil).GetMailboxesWithoutAttrib), arg0)
+}
+
+// GetSettings mocks base method.
+func (m *MockIMAPStateWrite) GetSettings(arg0 context.Context) (string, bool, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "GetSettings", arg0)
+	ret0, _ := ret[0].(string)
+	ret1, _ := ret[1].(bool)
+	ret2, _ := ret[2].(error)
+	return ret0, ret1, ret2
+}
+
+// GetSettings indicates an expected call of GetSettings.
+func (mr *MockIMAPStateWriteMockRecorder) GetSettings(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSettings", reflect.TypeOf((*MockIMAPStateWrite)(nil).GetSettings), arg0)
+}
+
+// PatchMailboxHierarchyWithoutTransforms mocks base method.
+func (m *MockIMAPStateWrite) PatchMailboxHierarchyWithoutTransforms(arg0 context.Context, arg1 imap.MailboxID, arg2 []string) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "PatchMailboxHierarchyWithoutTransforms", arg0, arg1, arg2)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// PatchMailboxHierarchyWithoutTransforms indicates an expected call of PatchMailboxHierarchyWithoutTransforms.
+func (mr *MockIMAPStateWriteMockRecorder) PatchMailboxHierarchyWithoutTransforms(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PatchMailboxHierarchyWithoutTransforms", reflect.TypeOf((*MockIMAPStateWrite)(nil).PatchMailboxHierarchyWithoutTransforms), arg0, arg1, arg2)
+}
+
+// StoreSettings mocks base method.
+func (m *MockIMAPStateWrite) StoreSettings(arg0 context.Context, arg1 string) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "StoreSettings", arg0, arg1)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// StoreSettings indicates an expected call of StoreSettings.
+func (mr *MockIMAPStateWriteMockRecorder) StoreSettings(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StoreSettings", reflect.TypeOf((*MockIMAPStateWrite)(nil).StoreSettings), arg0, arg1)
+}
+
+// UpdateMessageFlags mocks base method.
+func (m *MockIMAPStateWrite) UpdateMessageFlags(arg0 context.Context, arg1 imap.MessageID, arg2 imap.FlagSet) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "UpdateMessageFlags", arg0, arg1, arg2)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// UpdateMessageFlags indicates an expected call of UpdateMessageFlags.
+func (mr *MockIMAPStateWriteMockRecorder) UpdateMessageFlags(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateMessageFlags", reflect.TypeOf((*MockIMAPStateWrite)(nil).UpdateMessageFlags), arg0, arg1, arg2)
+}

internal/services/imapservice/service.go

@@ -158,7 +158,7 @@ func NewService(
 		syncUpdateApplier:  syncUpdateApplier,
 		syncMessageBuilder: syncMessageBuilder,
 		syncReporter:       syncReporter,
-		syncConfigPath:     getSyncConfigPath(syncConfigDir, identityState.User.ID),
+		syncConfigPath:     GetSyncConfigPath(syncConfigDir, identityState.User.ID),
 	}
 }
 
@@ -498,6 +498,7 @@ func (s *Service) buildConnectors() (map[string]*Connector, error) {
 			s.panicHandler,
 			s.telemetry,
 			s.showAllMail,
+			s.syncStateProvider,
 		)
 
 		return connectors, nil
@@ -514,6 +515,7 @@ func (s *Service) buildConnectors() (map[string]*Connector, error) {
 			s.panicHandler,
 			s.telemetry,
 			s.showAllMail,
+			s.syncStateProvider,
 		)
 	}
 
@@ -644,6 +646,6 @@ type setAddressModeReq struct {
 
 type getSyncFailedMessagesReq struct{}
 
-func getSyncConfigPath(path string, userID string) string {
+func GetSyncConfigPath(path string, userID string) string {
 	return filepath.Join(path, fmt.Sprintf("sync-%v", userID))
 }

internal/services/imapservice/service_address_events.go

@@ -128,6 +128,7 @@ func addNewAddressSplitMode(ctx context.Context, s *Service, addrID string) erro
 		s.panicHandler,
 		s.telemetry,
 		s.showAllMail,
+		s.syncStateProvider,
 	)
 
 	if err := s.serverManager.AddIMAPUser(ctx, connector, connector.addrID, s.gluonIDProvider, s.syncStateProvider); err != nil {

internal/services/imapservice/sync_state_provider.go

@@ -220,7 +220,7 @@ func (s *SyncState) loadUnsafe() error {
 }
 
 func DeleteSyncState(configDir, userID string) error {
-	path := getSyncConfigPath(configDir, userID)
+	path := GetSyncConfigPath(configDir, userID)
 
 	if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) {
 		return err
@@ -234,7 +234,7 @@ func MigrateVaultSettings(
 	hasLabels, hasMessages bool,
 	failedMessageIDs []string,
 ) (bool, error) {
-	filePath := getSyncConfigPath(configDir, userID)
+	filePath := GetSyncConfigPath(configDir, userID)
 
 	_, err := os.ReadFile(filePath) //nolint:gosec
 	if err == nil {

internal/services/imapservice/sync_state_provider_test.go

@@ -29,7 +29,7 @@ import (
 
 func TestMigrateSyncSettings_AlreadyExists(t *testing.T) {
 	tmpDir := t.TempDir()
-	testFile := getSyncConfigPath(tmpDir, "test")
+	testFile := GetSyncConfigPath(tmpDir, "test")
 
 	expected, err := generateTestState(testFile)
 	require.NoError(t, err)
@@ -53,7 +53,7 @@ func TestMigrateSyncSettings_DoesNotExist(t *testing.T) {
 	require.NoError(t, err)
 	require.True(t, migrated)
 
-	state, err := NewSyncState(getSyncConfigPath(tmpDir, "test"))
+	state, err := NewSyncState(GetSyncConfigPath(tmpDir, "test"))
 	require.NoError(t, err)
 	status, err := state.GetSyncStatus(context.Background())
 	require.NoError(t, err)

internal/services/imapservice/sync_update_applier.go

@@ -119,6 +119,10 @@ func (s *SyncUpdateApplier) SyncSystemLabelsOnly(ctx context.Context, labels map
 				continue
 			}
 
+			if label.Type != proton.LabelTypeSystem {
+				continue
+			}
+
 			for _, c := range connectors {
 				update := newSystemMailboxCreatedUpdate(imap.MailboxID(label.ID), label.Name)
 				updates = append(updates, update)

internal/services/imapsmtpserver/service.go

@@ -390,6 +390,11 @@ func (sm *Service) handleAddIMAPUserImpl(ctx context.Context,
 	} else {
 		log.Info("Creating new IMAP user")
 
+		// GODT-3003: Ensure previous IMAP sync state is cleared if we run into code path after vault corruption.
+		if err := syncStateProvider.ClearSyncStatus(ctx); err != nil {
+			return fmt.Errorf("failed to reset sync status: %w", err)
+		}
+
 		gluonID, err := sm.imapServer.AddUser(ctx, connector, idProvider.GluonKey())
 		if err != nil {
 			return fmt.Errorf("failed to add IMAP user: %w", err)

internal/services/syncservice/job.go

@@ -113,13 +113,14 @@ func (j *Job) onStageCompleted(ctx context.Context, count int64) {
 }
 
 func (j *Job) onJobFinished(ctx context.Context, lastMessageID string, count int64) {
-	defer j.wg.Done()
-
 	if err := j.state.SetLastMessageID(ctx, lastMessageID, count); err != nil {
 		j.log.WithError(err).Error("Failed to store last synced message id")
 		j.onError(err)
 		return
 	}
+
+	// j.onError() also calls j.wg.Done().
+	j.wg.Done()
 	j.syncReporter.OnProgress(ctx, count)
 }
 

internal/services/userevents/service.go

@@ -192,7 +192,6 @@ func (s *Service) run(ctx context.Context, lastEventID string) {
 	defer s.cpc.Close()
 	defer s.timer.Stop()
 	defer s.log.Info("Exiting service")
-	defer s.Close()
 
 	client := network.NewClientRetryWrapper(s.eventSource, &network.ExpCoolDown{})
 
@@ -303,14 +302,15 @@ func (s *Service) Close() {
 
 	// Cleanup pending removes.
 	for _, s := range s.pendingSubscriptions {
-		if s.op == pendingOpRemove {
 		if !processed.Contains(s.sub) {
+			processed.Add(s.sub)
+
+			if s.op == pendingOpRemove {
 				s.sub.close()
-			}
 			} else {
 				s.sub.cancel()
 				s.sub.close()
-			processed.Add(s.sub)
+			}
 		}
 	}
 

internal/user/user.go

@@ -589,6 +589,8 @@ func (user *User) Logout(ctx context.Context, withAPI bool) error {
 		return fmt.Errorf("failed to remove user from imap server: %w", err)
 	}
 
+	user.tasks.CancelAndWait()
+
 	// Stop Services
 	user.serviceGroup.CancelAndWait()
 
@@ -598,8 +600,6 @@ func (user *User) Logout(ctx context.Context, withAPI bool) error {
 	// Close imap service.
 	user.imapService.Close()
 
-	user.tasks.CancelAndWait()
-
 	if withAPI {
 		user.log.Debug("Logging out from API")
 
@@ -621,6 +621,9 @@ func (user *User) Logout(ctx context.Context, withAPI bool) error {
 func (user *User) Close() {
 	user.log.Info("Closing user")
 
+	// Stop any ongoing background tasks.
+	user.tasks.CancelAndWait()
+
 	// Stop Services
 	user.serviceGroup.CancelAndWait()
 
@@ -630,9 +633,6 @@ func (user *User) Close() {
 	// Close imap service.
 	user.imapService.Close()
 
-	// Stop any ongoing background tasks.
-	user.tasks.CancelAndWait()
-
 	// Close the user's API client.
 	user.client.Close()
 

pkg/files/removal.go

@@ -72,11 +72,12 @@ func remove(dir string, except ...string) error {
 
 	sort.Sort(sort.Reverse(sort.StringSlice(toRemove)))
 
+	var multiErr error
 	for _, target := range toRemove {
 		if err := os.RemoveAll(target); err != nil {
-			return err
+			multiErr = multierror.Append(multiErr, err)
 		}
 	}
 
-	return nil
+	return multiErr
 }

tests/features/imap/message/copy.feature

@@ -85,3 +85,18 @@ Feature: IMAP copy messages
       | from              | to                   | subject | unread |
       | john.doe@mail.com | [user:user]@[domain] | foo     | false  |
 
+  Scenario: Move message to trash then copy to folder does not delete message
+    When IMAP client "1" moves the message with subject "foo" from "INBOX" to "Trash"
+    And it succeeds
+    Then IMAP client "1" eventually sees the following messages in "Trash":
+      | from              | to                   | subject | unread |
+      | john.doe@mail.com | [user:user]@[domain] | foo     | false  |
+    When IMAP client "1" copies the message with subject "foo" from "Trash" to "Folders/mbox"
+    And it succeeds
+    When IMAP client "1" marks the message with subject "foo" as deleted
+    Then it succeeds
+    When IMAP client "1" expunges
+    Then it succeeds
+    Then IMAP client "1" eventually sees the following messages in "Folders/mbox":
+      | from              | to                   | subject | unread |
+      | john.doe@mail.com | [user:user]@[domain] | foo     | false  |

tests/features/imap/message/delete_from_trash.feature

@@ -7,7 +7,7 @@ Feature: IMAP remove messages from Trash
       | label | label  |
     Then it succeeds
 
-  Scenario Outline: Message in Trash and some other label is permanently deleted
+  Scenario Outline: Message in Trash and some other label is not permanently deleted
     Given the address "[user:user]@[domain]" of account "[user:user]" has the following messages in "Trash":
       | from              | to                   | subject | body  |
       | john.doe@mail.com | [user:user]@[domain] | foo     | hello |
@@ -27,8 +27,8 @@ Feature: IMAP remove messages from Trash
     When IMAP client "1" expunges
     Then it succeeds
     And IMAP client "1" eventually sees 1 messages in "Trash"
-    And IMAP client "1" eventually sees 1 messages in "All Mail"
-    And IMAP client "1" eventually sees 0 messages in "Labels/label"
+    And IMAP client "1" eventually sees 2 messages in "All Mail"
+    And IMAP client "1" eventually sees 1 messages in "Labels/label"
 
   Scenario Outline: Message in Trash only is permanently deleted
     Given the address "[user:user]@[domain]" of account "[user:user]" has the following messages in "Trash":