From 6ac8a4c0bcd336f61f0018ffa764bbca4de5a564 Mon Sep 17 00:00:00 2001
From: Leander Beernaert <leander.beernaert@proton.ch>
Date: Tue, 29 Nov 2022 07:28:09 -0800
Subject: [PATCH] GODT-2160: Ensure we can safely move cache file

It's currently impossible to wait until all SQLite write finish to disk.
This is not guaranteed when closing the ent DB client.

The existing code to move the cache handles the case where the
new location is on a new drive. However, due to the above issue this can
now lead to database corruption.

To avoid database corruption we now use the `os.Rename` function and
prevent moving the cache between drives until a better solution can be
implemented.
---
 internal/bridge/bridge_test.go | 28 ++++++++++++++++++++++------
 internal/bridge/files.go       | 26 +++-----------------------
 internal/bridge/settings.go    | 11 ++++++++++-
 3 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/internal/bridge/bridge_test.go b/internal/bridge/bridge_test.go
index 48698ed5..2c26fecb 100644
--- a/internal/bridge/bridge_test.go
+++ b/internal/bridge/bridge_test.go
@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"net/http"
 	"os"
+	"runtime"
 	"sync"
 	"testing"
 	"time"
@@ -455,11 +456,24 @@ func TestBridge_FactoryReset(t *testing.T) {
 	})
 }
 
-/* // This test will be enabled in a followup patch
+func TestBridge_ChangeCacheDirectoryFailsBetweenDifferentVolumes(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		t.Skip("Test only necessary on windows")
+	}
+	withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, vaultKey []byte) {
+		withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
+			// Change directory
+			err := bridge.SetGluonDir(ctx, "XX:\\")
+			require.Error(t, err)
+		})
+	})
+}
+
 func TestBridge_ChangeCacheDirectory(t *testing.T) {
 	withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, vaultKey []byte) {
 		withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
 			newCacheDir := t.TempDir()
+			currentCacheDir := bridge.GetGluonDir()
 
 			// Login the user.
 			userID, err := bridge.LoginFull(ctx, username, password, nil, nil)
@@ -470,14 +484,16 @@ func TestBridge_ChangeCacheDirectory(t *testing.T) {
 			require.Equal(t, []string{userID}, getConnectedUserIDs(t, bridge))
 
 			// Change directory
-			_ = bridge.SetGluonDir(ctx, newCacheDir)
+			err = bridge.SetGluonDir(ctx, newCacheDir)
+			require.NoError(t, err)
 
-			// The user is still there
-			//require.Equal(t, []string{userID}, bridge.GetUserIDs())
-			//require.Equal(t, []string{userID}, getConnectedUserIDs(t, bridge))
+			_, err = os.ReadDir(currentCacheDir)
+			require.True(t, os.IsNotExist(err))
+
+			require.Equal(t, newCacheDir, bridge.GetGluonDir())
 		})
 	})
-} */
+}
 
 // withEnv creates the full test environment and runs the tests.
 func withEnv(t *testing.T, tests func(context.Context, *server.Server, *proton.NetCtl, bridge.Locator, []byte), opts ...server.Option) {
diff --git a/internal/bridge/files.go b/internal/bridge/files.go
index 6c00cdfe..bb050e24 100644
--- a/internal/bridge/files.go
+++ b/internal/bridge/files.go
@@ -42,7 +42,7 @@ func moveDir(from, to string) error {
 				return err
 			}
 		} else {
-			if err := move(filepath.Join(from, entry.Name()), filepath.Join(to, entry.Name())); err != nil {
+			if err := moveFile(filepath.Join(from, entry.Name()), filepath.Join(to, entry.Name())); err != nil {
 				return err
 			}
 		}
@@ -51,32 +51,12 @@ func moveDir(from, to string) error {
 	return os.Remove(from)
 }
 
-func move(from, to string) error {
+func moveFile(from, to string) error {
 	if err := os.MkdirAll(filepath.Dir(to), 0o700); err != nil {
 		return err
 	}
 
-	f, err := os.Open(from) // nolint:gosec
-	if err != nil {
-		return err
-	}
-	defer func() { _ = f.Close() }()
-
-	c, err := os.Create(to) // nolint:gosec
-	if err != nil {
-		return err
-	}
-	defer func() { _ = f.Close() }()
-
-	if err := os.Chmod(to, 0o600); err != nil {
-		return err
-	}
-
-	if _, err := c.ReadFrom(f); err != nil {
-		return err
-	}
-
-	if err := os.Remove(from); err != nil {
+	if err := os.Rename(from, to); err != nil {
 		return err
 	}
 
diff --git a/internal/bridge/settings.go b/internal/bridge/settings.go
index 0734b56d..137b4ee9 100644
--- a/internal/bridge/settings.go
+++ b/internal/bridge/settings.go
@@ -21,6 +21,7 @@ import (
 	"context"
 	"fmt"
 	"net"
+	"path/filepath"
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/ProtonMail/proton-bridge/v3/internal/constants"
@@ -119,10 +120,18 @@ func (bridge *Bridge) GetGluonDir() string {
 
 func (bridge *Bridge) SetGluonDir(ctx context.Context, newGluonDir string) error {
 	return safe.RLockRet(func() error {
-		if newGluonDir == bridge.GetGluonDir() {
+		currentGluonDir := bridge.GetGluonDir()
+		if newGluonDir == currentGluonDir {
 			return fmt.Errorf("new gluon dir is the same as the old one")
 		}
 
+		currentVolumeName := filepath.VolumeName(currentGluonDir)
+		newVolumeName := filepath.VolumeName(newGluonDir)
+
+		if currentVolumeName != newVolumeName {
+			return fmt.Errorf("it's currently not possible to move the cache between different volumes")
+		}
+
 		if err := bridge.closeIMAP(context.Background()); err != nil {
 			return fmt.Errorf("failed to close IMAP: %w", err)
 		}