From ab919748764616dfd8740140aff9896528d376be Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Sun, 18 Oct 2026 11:30:45 +0200 Subject: [PATCH] fix(model): ensure parent ownership is copied properly (fixes #8399) (#10926) When creating multiple directories, inherit ownership all the way up (down). Signed-off-by: Jakob Borg --- lib/config/folderconfiguration.go | 4 ++++ lib/model/folder_sendrecv.go | 38 +++++++++++++++++++++++++++---- lib/model/folder_sendrecv_test.go | 25 ++++++++++++++++++++ 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/lib/config/folderconfiguration.go b/lib/config/folderconfiguration.go index 58c244e42..a08a7ea48 100644 --- a/lib/config/folderconfiguration.go +++ b/lib/config/folderconfiguration.go @@ -323,6 +323,10 @@ func (f *FolderConfiguration) prepare(myID protocol.DeviceID, existingDevices ma if f.Type == FolderTypeReceiveEncrypted { f.IgnorePerms = true } + + if f.SyncOwnership { + f.CopyOwnershipFromParent = false + } } // RequiresRestartOnly returns a copy with only the attributes that require diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go index 9d5252624..9902dc572 100644 --- a/lib/model/folder_sendrecv.go +++ b/lib/model/folder_sendrecv.go @@ -670,10 +670,10 @@ func (f *sendReceiveFolder) handleDir(file protocol.FileInfo, dbUpdateChan chan< f.newPullError(file.Name, fmt.Errorf("handling dir (setting permissions): %w", err)) return } - if err := f.setPlatformData(&file, file.Name); err != nil { - f.newPullError(file.Name, fmt.Errorf("handling dir (setting metadata): %w", err)) - return - } + } + if err := f.setPlatformData(&file, file.Name); err != nil { + f.newPullError(file.Name, fmt.Errorf("handling dir (setting metadata): %w", err)) + return } dbUpdateChan <- dbUpdateJob{file, dbUpdateHandleDir} } @@ -708,7 +708,7 @@ func (f *sendReceiveFolder) checkParent(file string, scanChan chan<- string) boo return true } f.sl.Debug("Creating parent directory", slogutil.FilePath(file)) - if err := f.mtimefs.MkdirAll(parent, fs.ModePerm); err != nil { + if err := f.mkdirAllWithOwnership(parent); err != nil { f.newPullError(file, fmt.Errorf("creating parent dir: %w", err)) return false } @@ -718,6 +718,34 @@ func (f *sendReceiveFolder) checkParent(file string, scanChan chan<- string) boo return true } +// mkdirAllWithOwnership creates missing parents from the top down so each +// directory inherits the ownership of its parent before creating children. +func (f *sendReceiveFolder) mkdirAllWithOwnership(path string) error { + if !f.CopyOwnershipFromParent || build.IsWindows { + return f.mtimefs.MkdirAll(path, fs.ModePerm) + } + + if _, err := f.mtimefs.Lstat(path); !fs.IsNotExist(err) { + // Let MkdirAll check that an existing path is a directory. + return f.mtimefs.MkdirAll(path, fs.ModePerm) + } + parent := filepath.Dir(path) + if parent == path { + return fs.ErrNotExist + } + if err := f.mkdirAllWithOwnership(parent); err != nil { + return err + } + if err := f.mtimefs.Mkdir(path, fs.ModePerm); err != nil { + if fs.IsExist(err) { + // Another process may have created it. Preserve its ownership. + return f.mtimefs.MkdirAll(path, fs.ModePerm) + } + return err + } + return f.copyOwnershipFromParent(path) +} + // handleSymlink creates or updates the given symlink func (f *sendReceiveFolder) handleSymlink(file protocol.FileInfo, dbUpdateChan chan<- dbUpdateJob, scanChan chan<- string) { // Used in the defer closure below, updated by the function body. Take diff --git a/lib/model/folder_sendrecv_test.go b/lib/model/folder_sendrecv_test.go index 108226089..d57f8b9e1 100644 --- a/lib/model/folder_sendrecv_test.go +++ b/lib/model/folder_sendrecv_test.go @@ -774,6 +774,31 @@ func TestCopyOwner(t *testing.T) { } } +func TestCopyOwnerMissingParents(t *testing.T) { + if build.IsWindows { + t.Skip("copying owner not supported on Windows") + } + + m, f := setupSendReceiveFolder(t) + f.folder.FolderConfiguration = newFolderConfiguration(m.cfg, f.ID, f.Label, config.FilesystemTypeFake, "/TestCopyOwnerMissingParents") + f.CopyOwnershipFromParent = true + must(t, f.mtimefs.Lchown(".", "1111", "2222")) + must(t, f.mtimefs.Mkdir("existing", 0o755)) + must(t, f.mtimefs.Lchown("existing", "1234", "5678")) + + scanChan := make(chan string, 1) + if !f.checkParent(filepath.FromSlash("existing/one/two/file"), scanChan) { + t.Fatal("creating missing parents failed") + } + for _, name := range []string{"existing", "existing/one", "existing/one/two"} { + info, err := f.mtimefs.Lstat(name) + must(t, err) + if info.Owner() != 1234 || info.Group() != 5678 { + t.Errorf("%s: expected owner/group 1234/5678, got %d/%d", name, info.Owner(), info.Group()) + } + } +} + // TestSRConflictReplaceFileByDir checks that a conflict is created when an existing file // is replaced with a directory and versions are conflicting func TestSRConflictReplaceFileByDir(t *testing.T) {