瀏覽代碼

fix(versioner): fix perms of created folders (fixes #9626) (#10105)

As suggested in the linked issue, I've updated the versioner code to use
the permissions of the corresponding directory in the synced folder,
when creating the folder in the versions directory

### Testing
- Some tests are included with the PR. Happy to add more if you think
there are some edge-cases that we're missing.
- I've tested manually on linux to confirm the permissions of the
created directories.
- I haven't tested on Windows or OSX (I don't have access to these OS)
Ashish Bhate 4 月之前
父節點
當前提交
1a131a56f2
共有 2 個文件被更改,包括 147 次插入3 次删除
  1. 100 0
      lib/versioner/simple_test.go
  2. 47 3
      lib/versioner/util.go

+ 100 - 0
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)
+	}
+}

+ 47 - 3
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)