diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 195b302de..51a8a715f 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -295,10 +295,12 @@ func (c *serveCmd) Run() error { } } - // Ensure that our home directory exists. - if err := syncthing.EnsureDir(locations.GetBaseDir(locations.ConfigBaseDir), 0o700); err != nil { - l.Warnln("Failure on home directory:", err) - os.Exit(svcutil.ExitError.AsInt()) + // Ensure that our config and data directories exist. + for _, loc := range []locations.BaseDirEnum{locations.ConfigBaseDir, locations.DataBaseDir} { + if err := syncthing.EnsureDir(locations.GetBaseDir(loc), 0o700); err != nil { + l.Warnln("Failed to ensure directory exists:", err) + os.Exit(svcutil.ExitError.AsInt()) + } } if c.InternalInnerProcess { diff --git a/lib/model/folder_test.go b/lib/model/folder_test.go index a09b5af30..f73c665ff 100644 --- a/lib/model/folder_test.go +++ b/lib/model/folder_test.go @@ -8,6 +8,7 @@ package model import ( "path/filepath" + "slices" "testing" "github.com/d4l3k/messagediff" @@ -117,20 +118,11 @@ func unifySubsCases() []unifySubsCase { return cases } -func unifyExists(f string, tc unifySubsCase) bool { - for _, e := range tc.exists { - if f == e { - return true - } - } - return false -} - func TestUnifySubs(t *testing.T) { cases := unifySubsCases() for i, tc := range cases { exists := func(f string) bool { - return unifyExists(f, tc) + return slices.Contains(tc.exists, f) } out := unifySubs(tc.in, exists) if diff, equal := messagediff.PrettyDiff(tc.out, out); !equal { @@ -146,7 +138,7 @@ func BenchmarkUnifySubs(b *testing.B) { for i := 0; i < b.N; i++ { for _, tc := range cases { exists := func(f string) bool { - return unifyExists(f, tc) + return slices.Contains(tc.exists, f) } unifySubs(tc.in, exists) } diff --git a/lib/model/model.go b/lib/model/model.go index e9fa0b1d3..1d43bddd1 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -1817,11 +1817,9 @@ func (m *model) handleAutoAccepts(deviceID protocol.DeviceID, folder protocol.Fo l.Infof("Failed to auto-accept folder %s from %s due to path conflict", folder.Description(), deviceID) return config.FolderConfiguration{}, false } else { - for _, device := range cfg.DeviceIDs() { - if device == deviceID { - // Already shared nothing todo. - return config.FolderConfiguration{}, false - } + if slices.Contains(cfg.DeviceIDs(), deviceID) { + // Already shared nothing todo. + return config.FolderConfiguration{}, false } if cfg.Type == config.FolderTypeReceiveEncrypted { if len(ccDeviceInfos.remote.EncryptionPasswordToken) == 0 && len(ccDeviceInfos.local.EncryptionPasswordToken) == 0 { diff --git a/lib/model/model_test.go b/lib/model/model_test.go index c4ff6e100..268d19eef 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -18,6 +18,7 @@ import ( "os" "path/filepath" "runtime/pprof" + "slices" "sort" "strconv" "strings" @@ -1254,10 +1255,8 @@ func TestAutoAcceptPausedWhenFolderConfigChanged(t *testing.T) { } else if fcfg.Path != idOther { t.Error("folder path changed") } else { - for _, dev := range fcfg.DeviceIDs() { - if dev == device1 { - return - } + if slices.Contains(fcfg.DeviceIDs(), device1) { + return } t.Error("device missing") } @@ -1303,10 +1302,8 @@ func TestAutoAcceptPausedWhenFolderConfigNotChanged(t *testing.T) { } else if fcfg.Path != idOther { t.Error("folder path changed") } else { - for _, dev := range fcfg.DeviceIDs() { - if dev == device1 { - return - } + if slices.Contains(fcfg.DeviceIDs(), device1) { + return } t.Error("device missing") } diff --git a/lib/versioner/simple_test.go b/lib/versioner/simple_test.go index affcf3081..00527a3bb 100644 --- a/lib/versioner/simple_test.go +++ b/lib/versioner/simple_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + "github.com/syncthing/syncthing/lib/build" "github.com/syncthing/syncthing/lib/config" ) @@ -156,3 +157,102 @@ func TestPathTildes(t *testing.T) { t.Fatalf("found versioned file %q, want one that begins with %q", got, testPath) } } + +func TestArchiveFoldersCreationPermission(t *testing.T) { + if build.IsWindows { + t.Skip("Skipping on Windows") + return + } + dir := t.TempDir() + versionsDir := t.TempDir() + + cfg := config.FolderConfiguration{ + FilesystemType: config.FilesystemTypeBasic, + Path: dir, + Versioning: config.VersioningConfiguration{ + FSPath: versionsDir, + FSType: config.FilesystemTypeBasic, + Params: map[string]string{ + "keep": "2", + }, + }, + } + vfs := cfg.Filesystem(nil) + v := newSimple(cfg) + + // Create two folders and set their permissions + folder1Path := filepath.Join(dir, "folder1") + folder1Perms := os.FileMode(0o777) + folder1VersionsPath := filepath.Join(versionsDir, "folder1") + err := os.Mkdir(folder1Path, folder1Perms) + if err != nil { + t.Fatal(err) + } + // chmod incase umask changes the create permissions + err = os.Chmod(folder1Path, folder1Perms) + if err != nil { + t.Fatal(err) + } + + folder2Path := filepath.Join(folder1Path, "földer2") + folder2VersionsPath := filepath.Join(folder1VersionsPath, "földer2") + folder2Perms := os.FileMode(0o744) + err = os.Mkdir(folder2Path, folder2Perms) + if err != nil { + t.Fatal(err) + } + // chmod incase umask changes the create permissions + err = os.Chmod(folder2Path, folder2Perms) + if err != nil { + t.Fatal(err) + } + + // create a file + filePath := filepath.Join("folder1", "földer2", "testFile") + f, err := vfs.Create(filePath) + if err != nil { + t.Fatal(err) + } + f.Close() + + if err := v.Archive(filePath); err != nil { + t.Error(err) + } + + // check permissions of the created version folders + folder1VersionsInfo, err := os.Stat(folder1VersionsPath) + if err != nil { + t.Fatal(err) + } + if folder1VersionsInfo.Mode().Perm() != folder1Perms { + t.Errorf("folder1 permissions %v, want %v", folder1VersionsInfo.Mode(), folder1Perms) + } + + folder2VersionsInfo, err := os.Stat(folder2VersionsPath) + if err != nil { + t.Fatal(err) + } + if folder2VersionsInfo.Mode().Perm() != folder2Perms { + t.Errorf("földer2 permissions %v, want %v", folder2VersionsInfo.Mode(), folder2Perms) + } + + // Archive again to test that archiving doesn't fail if the versioned folders already exist + if err := v.Archive(filePath); err != nil { + t.Error(err) + } + folder1VersionsInfo, err = os.Stat(folder1VersionsPath) + if err != nil { + t.Fatal(err) + } + if folder1VersionsInfo.Mode().Perm() != folder1Perms { + t.Errorf("folder1 permissions %v, want %v", folder1VersionsInfo.Mode(), folder1Perms) + } + + folder2VersionsInfo, err = os.Stat(folder2VersionsPath) + if err != nil { + t.Fatal(err) + } + if folder2VersionsInfo.Mode().Perm() != folder2Perms { + t.Errorf("földer2 permissions %v, want %v", folder2VersionsInfo.Mode(), folder2Perms) + } +} diff --git a/lib/versioner/util.go b/lib/versioner/util.go index 41b4aaf32..3a5c97df1 100644 --- a/lib/versioner/util.go +++ b/lib/versioner/util.go @@ -10,6 +10,7 @@ import ( "context" "errors" "fmt" + "os" "path/filepath" "regexp" "sort" @@ -164,9 +165,8 @@ func archiveFile(method fs.CopyRangeMethod, srcFs, dstFs fs.Filesystem, filePath file := filepath.Base(filePath) inFolderPath := filepath.Dir(filePath) - - err = dstFs.MkdirAll(inFolderPath, 0o755) - if err != nil && !fs.IsExist(err) { + err = dupDirTree(srcFs, dstFs, inFolderPath) + if err != nil { l.Debugln("archiving", filePath, err) return err } @@ -190,6 +190,50 @@ func archiveFile(method fs.CopyRangeMethod, srcFs, dstFs fs.Filesystem, filePath return err } +func dupDirTree(srcFs, dstFs fs.Filesystem, folderPath string) error { + // Return early if the folder already exists. + _, err := dstFs.Stat(folderPath) + if err == nil || !fs.IsNotExist(err) { + return err + } + hadParent := true + for i := range folderPath { + if os.IsPathSeparator(folderPath[i]) { + // If the parent folder didn't exist, then this folder doesn't exist + // so we can skip the check + if hadParent { + _, err := dstFs.Stat(folderPath[:i]) + if err == nil { + continue + } + if !fs.IsNotExist(err) { + return err + } + } + hadParent = false + err := dupDirWithPerms(srcFs, dstFs, folderPath[:i]) + if err != nil { + return err + } + } + } + return dupDirWithPerms(srcFs, dstFs, folderPath) +} + +func dupDirWithPerms(srcFs, dstFs fs.Filesystem, folderPath string) error { + srcStat, err := srcFs.Stat(folderPath) + if err != nil { + return err + } + // If we call Mkdir with srcStat.Mode(), we won't get the expected perms because of umask + // So, we create the folder with 0700, and then change the perms to the srcStat.Mode() + err = dstFs.Mkdir(folderPath, 0o700) + if err != nil { + return err + } + return dstFs.Chmod(folderPath, srcStat.Mode()) +} + func restoreFile(method fs.CopyRangeMethod, src, dst fs.Filesystem, filePath string, versionTime time.Time, tagger fileTagger) error { tag := versionTime.In(time.Local).Truncate(time.Second).Format(TimeFormat) taggedFilePath := tagger(filePath, tag)