Repository: syncthing/syncthing (language: go)

Issue:
Upgrading from v1.19.2 to v1.20.x now requires chmod for syncing files
After upgrading from v1.19.2 to v1.20.1 syncing of files fails with `syncing: chmod ...: operation not permitted` when trying to write data changes.

This was working in v1.19.2 with Restricted ACL mode for the data directory. I would rather not remove Restricted mode as it breaks other programs as it's a SMB share location.

Setup:
- Syncthing v1.20.1, Linux (64-bit Intel/AMD)
- Running on TrueNAS-SCALE-22.02.1 as a docker image
- The data directory is a ZFS dataset with Restricted ACL mode enabled
- The syncthing user account has read and write access to the data directory



Discussion / hints:
Syncthing always chmods files as part of syncing them. Perhaps you want to set the "ignore permissions" option? 
The "ignore permissions" option was and is set.
Interesting. Can you post a real log with more context?
The only thing that shows up in the log is `syncing: chmod ...: operation not permitted` and it trying after a few minutes.
Yeah I don't see how that can happen, so without a full log and ideally corresponding config I'm lost.
Maybe related?

https://github.com/syncthing/syncthing/pull/8235

This seems to be the only recent change related to `chmod`.
Yes, that's likely it @tomasz1986 . That mistakenly changed behaviour (error returned on chmod instead of only on the actual operation), while it's actual purpose was to change the permission bits. I'll revert those bits.

Fix patch:
diff --git a/lib/model/util.go b/lib/model/util.go
index 9309f1a94b0..a746fecfe43 100644
--- a/lib/model/util.go
+++ b/lib/model/util.go
@@ -116,28 +116,35 @@ func inWritableDir(fn func(string) error, targetFs fs.Filesystem, path string, i
 	}
 
 	const permBits = fs.ModePerm | fs.ModeSetuid | fs.ModeSetgid | fs.ModeSticky
+	var parentErr error
 	if mode := info.Mode() & permBits; mode&0200 == 0 {
 		// A non-writeable directory (for this user; we assume that's the
 		// relevant part). Temporarily change the mode so we can delete the
 		// file or directory inside it.
-		if err := targetFs.Chmod(dir, mode|0700); err != nil {
-			return err
-		}
-		// Chmod succeeded, we should change the permissions back on the way
-		// out. If we fail we log the error as we have irrevocably messed up
-		// at this point. :( (The operation we were called to wrap has
-		// succeeded or failed on its own so returning an error to the
-		// caller is inappropriate.)
-		defer func() {
-			if err := targetFs.Chmod(dir, mode); err != nil && !fs.IsNotExist(err) {
-				logFn := l.Warnln
-				if ignorePerms {
-					logFn = l.Debugln
+		parentErr = targetFs.Chmod(dir, mode|0700)
+		if parentErr != nil {
+			l.Debugf("Failed to make parent directory writable: %v", parentErr)
+		} else {
+			// Chmod succeeded, we should change the permissions back on the way
+			// out. If we fail we log the error as we have irrevocably messed up
+			// at this point. :( (The operation we were called to wrap has
+			// succeeded or failed on its own so returning an error to the
+			// caller is inappropriate.)
+			defer func() {
+				if err := targetFs.Chmod(dir, mode); err != nil && !fs.IsNotExist(err) {
+					logFn := l.Warnln
+					if ignorePerms {
+						logFn = l.Debugln
+					}
+					logFn("Failed to restore directory permissions after gaining write access:", err)
 				}
-				logFn("Failed to restore directory permissions after gaining write access:", err)
-			}
-		}()
+			}()
+		}
 	}
 
-	return fn(path)
+	err = fn(path)
+	if fs.IsPermission(err) && parentErr != nil {
+		err = fmt.Errorf("error after failing to make parent directory writable: %w", err)
+	}
+	return err
 }


Test patch: