From 6df85dc95c5cd7d02834c9dce322814c6bf26a24 Mon Sep 17 00:00:00 2001
From: Jakob Borg <jakob@kastelo.net>
Date: Wed, 3 Jun 2026 10:54:04 +0200
Subject: [PATCH] fix: let umask do the thing (#10723)

We had a few places where we had perhaps too much of an opinion on the
permissions on created files and directories, sometimes fuled by a
misconception about how permissions work in both Unix and Windows. Recap
on the ground rules:

- On all unixes, all file & directory creation (`Mkdir`, `MkdirAll`,
`Create`, `WriteFile`, `Open`) has the given permission bits filtered
via the user's umask. The proper permissions for us to use are in almost
all cases 0o666 for files and 0o777 for directories, strange as that may
look at the call site.
- On Windows, there is no umask but in turn all of the permission bits
except the user write bit are ignored. The absence of user write bit is
converted into the read only attribute. This means that what is proper
for Unix above is also proper for Windows.
- We make an exception when creating files for certificate keys and the
config / database directories, as those contain secrets we think should remain closed
even if the user generally collaborates with other users on the system.

(Also removal of a bugfixed copy of MkdirAll for Windows that hasn't
been necessary for a few years.)

---------

Signed-off-by: Jakob Borg <jakob@kastelo.net>
---
 build.go                               |  6 +--
 cmd/infra/stcrashreceiver/diskstore.go |  6 +--
 cmd/infra/stcrashreceiver/util.go      |  2 +-
 cmd/infra/strelaypoolsrv/main.go       |  2 +-
 cmd/syncthing/decrypt/decrypt.go       |  2 +-
 cmd/syncthing/main.go                  |  2 +-
 cmd/syncthing/monitor.go               |  2 +-
 lib/api/api.go                         |  2 +-
 lib/config/folderconfiguration.go      | 15 ++------
 lib/config/migrations.go               | 20 +++-------
 lib/fs/basicfs.go                      |  2 +-
 lib/fs/basicfs_unix.go                 |  4 --
 lib/fs/basicfs_windows.go              | 51 --------------------------
 lib/fs/fakefs.go                       |  4 +-
 lib/model/folder_sendrecv.go           |  2 +-
 lib/model/model.go                     |  2 +-
 lib/versioner/util.go                  |  4 +-
 script/authors.go                      |  2 +-
 script/copyrights.go                   |  2 +-
 19 files changed, 30 insertions(+), 102 deletions(-)

diff --git a/build.go b/build.go
index 779b6a27e..fdb6fc481 100644
--- a/build.go
+++ b/build.go
@@ -718,7 +718,7 @@ func shouldBuildSyso(dir string) (string, error) {
 	}
 
 	jsonPath := filepath.Join(dir, "versioninfo.json")
-	err = os.WriteFile(jsonPath, bs, 0o644)
+	err = os.WriteFile(jsonPath, bs, 0o666)
 	if err != nil {
 		return "", errors.New("failed to create " + jsonPath + ": " + err.Error())
 	}
@@ -783,7 +783,7 @@ func copyFile(src, dst string, perm os.FileMode) error {
 	}
 
 copy:
-	os.MkdirAll(filepath.Dir(dst), 0o777)
+	os.MkdirAll(filepath.Dir(dst), os.ModePerm)
 	if err := os.WriteFile(dst, in, perm); err != nil {
 		return err
 	}
@@ -1432,7 +1432,7 @@ func writeCompatJSON() {
 			continue
 		}
 		bs, _ := json.MarshalIndent(e, "", "  ")
-		if err := os.WriteFile("compat.json", bs, 0o644); err != nil {
+		if err := os.WriteFile("compat.json", bs, 0o666); err != nil {
 			log.Fatal("Writing compat.json:", err)
 		}
 		return
diff --git a/cmd/infra/stcrashreceiver/diskstore.go b/cmd/infra/stcrashreceiver/diskstore.go
index 2e3788be7..e13c8bad8 100644
--- a/cmd/infra/stcrashreceiver/diskstore.go
+++ b/cmd/infra/stcrashreceiver/diskstore.go
@@ -42,7 +42,7 @@ type currentFile struct {
 }
 
 func (d *diskStore) Serve(ctx context.Context) {
-	if err := os.MkdirAll(d.dir, 0o700); err != nil {
+	if err := os.MkdirAll(d.dir, os.ModePerm); err != nil {
 		log.Println("Creating directory:", err)
 		return
 	}
@@ -62,7 +62,7 @@ func (d *diskStore) Serve(ctx context.Context) {
 		case entry := <-d.inbox:
 			path := d.fullPath(entry.path)
 
-			if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
+			if err := os.MkdirAll(filepath.Dir(path), os.ModePerm); err != nil {
 				log.Println("Creating directory:", err)
 				continue
 			}
@@ -77,7 +77,7 @@ func (d *diskStore) Serve(ctx context.Context) {
 				log.Println("Failed to compress crash report:", err)
 				continue
 			}
-			if err := os.WriteFile(path, buf.Bytes(), 0o600); err != nil {
+			if err := os.WriteFile(path, buf.Bytes(), 0o666); err != nil {
 				log.Printf("Failed to write %s: %v", entry.path, err)
 				_ = os.Remove(path)
 				continue
diff --git a/cmd/infra/stcrashreceiver/util.go b/cmd/infra/stcrashreceiver/util.go
index ad58b5caa..f0d5b4ccc 100644
--- a/cmd/infra/stcrashreceiver/util.go
+++ b/cmd/infra/stcrashreceiver/util.go
@@ -52,5 +52,5 @@ func compressAndWrite(bs []byte, fullPath string) error {
 	gw.Close()
 
 	// Create an output file with the compressed report
-	return os.WriteFile(fullPath, buf.Bytes(), 0o644)
+	return os.WriteFile(fullPath, buf.Bytes(), 0o666)
 }
diff --git a/cmd/infra/strelaypoolsrv/main.go b/cmd/infra/strelaypoolsrv/main.go
index 0871d5982..fee62dbda 100644
--- a/cmd/infra/strelaypoolsrv/main.go
+++ b/cmd/infra/strelaypoolsrv/main.go
@@ -612,7 +612,7 @@ func saveRelays(file string, relays []*relay) error {
 	for _, relay := range relays {
 		content += relay.uri.String() + "\n"
 	}
-	return os.WriteFile(file, []byte(content), 0o777)
+	return os.WriteFile(file, []byte(content), 0o666)
 }
 
 func createTestCertificate() tls.Certificate {
diff --git a/cmd/syncthing/decrypt/decrypt.go b/cmd/syncthing/decrypt/decrypt.go
index 4545e119c..862566a85 100644
--- a/cmd/syncthing/decrypt/decrypt.go
+++ b/cmd/syncthing/decrypt/decrypt.go
@@ -167,7 +167,7 @@ func (c *CLI) process(srcFs fs.Filesystem, dstFs fs.Filesystem, path string) err
 
 	var plainFd fs.File
 	if dstFs != nil {
-		if err := dstFs.MkdirAll(filepath.Dir(plainFi.Name), 0o700); err != nil {
+		if err := dstFs.MkdirAll(filepath.Dir(plainFi.Name), fs.ModePerm); err != nil {
 			return fmt.Errorf("%s: %w", plainFi.Name, err)
 		}
 
diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go
index bd35c6667..35ed25c34 100644
--- a/cmd/syncthing/main.go
+++ b/cmd/syncthing/main.go
@@ -673,7 +673,7 @@ func auditWriter(auditFile string) io.Writer {
 		} else {
 			auditFlags = os.O_WRONLY | os.O_CREATE | os.O_APPEND
 		}
-		fd, err = os.OpenFile(auditFile, auditFlags, 0o600)
+		fd, err = os.OpenFile(auditFile, auditFlags, 0o666)
 		if err != nil {
 			slog.Error("Failed to open audit file", slogutil.Error(err))
 			os.Exit(svcutil.ExitError.AsInt())
diff --git a/cmd/syncthing/monitor.go b/cmd/syncthing/monitor.go
index 35f98600b..7b0cf2186 100644
--- a/cmd/syncthing/monitor.go
+++ b/cmd/syncthing/monitor.go
@@ -479,7 +479,7 @@ func (f *autoclosedFile) ensureOpenLocked() error {
 	// We open the file for write only, and create it if it doesn't exist.
 	flags := os.O_WRONLY | os.O_CREATE | os.O_APPEND
 
-	fd, err := os.OpenFile(f.name, flags, 0o644)
+	fd, err := os.OpenFile(f.name, flags, 0o666)
 	if err != nil {
 		return err
 	}
diff --git a/lib/api/api.go b/lib/api/api.go
index dfdfacc36..046ad33f2 100644
--- a/lib/api/api.go
+++ b/lib/api/api.go
@@ -1248,7 +1248,7 @@ func (s *service) getSupportBundle(w http.ResponseWriter, r *http.Request) {
 	zipFilePath := filepath.Join(locations.GetBaseDir(locations.ConfigBaseDir), zipFileName)
 
 	// Write buffer zip to local zip file (back up)
-	if err := os.WriteFile(zipFilePath, zipFilesBuffer.Bytes(), 0o600); err != nil {
+	if err := os.WriteFile(zipFilePath, zipFilesBuffer.Bytes(), 0o666); err != nil {
 		slog.Warn("Failed to create support bundle zip (file)", slogutil.FilePath(zipFilePath), slogutil.Error(err))
 	}
 
diff --git a/lib/config/folderconfiguration.go b/lib/config/folderconfiguration.go
index 10c8a2776..a2ed4e5ae 100644
--- a/lib/config/folderconfiguration.go
+++ b/lib/config/folderconfiguration.go
@@ -168,7 +168,7 @@ func (f *FolderConfiguration) CreateMarker() error {
 	ffs := f.Filesystem()
 
 	// Create the marker as a directory
-	err := ffs.Mkdir(DefaultMarkerName, 0o755)
+	err := ffs.Mkdir(DefaultMarkerName, fs.ModePerm)
 	if err != nil {
 		return err
 	}
@@ -176,7 +176,7 @@ func (f *FolderConfiguration) CreateMarker() error {
 	// Create a file inside it, reducing the risk of the marker directory
 	// being removed by automated cleanup tools.
 	markerFile := filepath.Join(DefaultMarkerName, f.markerFilename())
-	if err := fs.WriteFile(ffs, markerFile, f.markerContents(), 0o644); err != nil {
+	if err := fs.WriteFile(ffs, markerFile, f.markerContents(), 0o666); err != nil {
 		return err
 	}
 
@@ -246,19 +246,10 @@ func (f *FolderConfiguration) checkFilesystemPath(ffs fs.Filesystem, path string
 }
 
 func (f *FolderConfiguration) CreateRoot() (err error) {
-	// Directory permission bits. Will be filtered down to something
-	// sane by umask on Unixes.
-	permBits := fs.FileMode(0o777)
-	if build.IsWindows {
-		// Windows has no umask so we must chose a safer set of bits to
-		// begin with.
-		permBits = 0o700
-	}
-
 	filesystem := f.Filesystem()
 
 	if _, err = filesystem.Stat("."); fs.IsNotExist(err) {
-		err = filesystem.MkdirAll(".", permBits)
+		err = filesystem.MkdirAll(".", fs.ModePerm)
 	}
 
 	return err
diff --git a/lib/config/migrations.go b/lib/config/migrations.go
index 382bccfad..d237e8a00 100644
--- a/lib/config/migrations.go
+++ b/lib/config/migrations.go
@@ -18,7 +18,6 @@ import (
 	"sync"
 
 	"github.com/syncthing/syncthing/internal/slogutil"
-	"github.com/syncthing/syncthing/lib/build"
 	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/netutil"
 	"github.com/syncthing/syncthing/lib/upgrade"
@@ -224,27 +223,20 @@ func migrateToConfigV24(cfg *Configuration) {
 }
 
 func migrateToConfigV23(cfg *Configuration) {
-	permBits := fs.FileMode(0o777)
-	if build.IsWindows {
-		// Windows has no umask so we must chose a safer set of bits to
-		// begin with.
-		permBits = 0o700
-	}
-
 	// Upgrade code remains hardcoded for .stfolder despite configurable
 	// marker name in later versions.
 
 	for i := range cfg.Folders {
-		fs := cfg.Folders[i].Filesystem()
+		ffs := cfg.Folders[i].Filesystem()
 		// Invalid config posted, or tests.
-		if fs == nil {
+		if ffs == nil {
 			continue
 		}
-		if stat, err := fs.Stat(DefaultMarkerName); err == nil && !stat.IsDir() {
-			err = fs.Remove(DefaultMarkerName)
+		if stat, err := ffs.Stat(DefaultMarkerName); err == nil && !stat.IsDir() {
+			err = ffs.Remove(DefaultMarkerName)
 			if err == nil {
-				err = fs.Mkdir(DefaultMarkerName, permBits)
-				fs.Hide(DefaultMarkerName) // ignore error
+				err = ffs.Mkdir(DefaultMarkerName, fs.ModePerm)
+				ffs.Hide(DefaultMarkerName) // ignore error
 			}
 			if err != nil {
 				slog.Warn("Failed to upgrade folder marker", slogutil.Error(err))
diff --git a/lib/fs/basicfs.go b/lib/fs/basicfs.go
index d5a01184f..cf40f6fc8 100644
--- a/lib/fs/basicfs.go
+++ b/lib/fs/basicfs.go
@@ -174,7 +174,7 @@ func (f *BasicFilesystem) MkdirAll(path string, perm FileMode) error {
 		return err
 	}
 
-	return f.mkdirAll(path, os.FileMode(perm))
+	return os.MkdirAll(path, os.FileMode(perm))
 }
 
 func (f *BasicFilesystem) Lstat(name string) (FileInfo, error) {
diff --git a/lib/fs/basicfs_unix.go b/lib/fs/basicfs_unix.go
index c60e67574..dc0f963ce 100644
--- a/lib/fs/basicfs_unix.go
+++ b/lib/fs/basicfs_unix.go
@@ -32,10 +32,6 @@ func (f *BasicFilesystem) ReadSymlink(name string) (string, error) {
 	return os.Readlink(name)
 }
 
-func (*BasicFilesystem) mkdirAll(path string, perm os.FileMode) error {
-	return os.MkdirAll(path, perm)
-}
-
 // Unhide is a noop on unix, as unhiding files requires renaming them.
 // We still check that the relative path does not try to escape the root
 func (f *BasicFilesystem) Unhide(name string) error {
diff --git a/lib/fs/basicfs_windows.go b/lib/fs/basicfs_windows.go
index d1b8d88c2..34c3bb860 100644
--- a/lib/fs/basicfs_windows.go
+++ b/lib/fs/basicfs_windows.go
@@ -31,57 +31,6 @@ func (BasicFilesystem) CreateSymlink(target, name string) error {
 	return errNotSupported
 }
 
-// Required due to https://github.com/golang/go/issues/10900
-func (f *BasicFilesystem) mkdirAll(path string, perm os.FileMode) error {
-	// Fast path: if we can tell whether path is a directory or file, stop with success or error.
-	dir, err := os.Stat(path)
-	if err == nil {
-		if dir.IsDir() {
-			return nil
-		}
-		return &os.PathError{
-			Op:   "mkdir",
-			Path: path,
-			Err:  syscall.ENOTDIR,
-		}
-	}
-
-	// Slow path: make sure parent exists and then call Mkdir for path.
-	i := len(path)
-	for i > 0 && IsPathSeparator(path[i-1]) { // Skip trailing path separator.
-		i--
-	}
-
-	j := i
-	for j > 0 && !IsPathSeparator(path[j-1]) { // Scan backward over element.
-		j--
-	}
-
-	if j > 1 {
-		// Create parent
-		parent := path[0 : j-1]
-		if parent != filepath.VolumeName(parent) {
-			err = f.mkdirAll(parent, perm)
-			if err != nil {
-				return err
-			}
-		}
-	}
-
-	// Parent now exists; invoke Mkdir and use its result.
-	err = os.Mkdir(path, perm)
-	if err != nil {
-		// Handle arguments like "foo/." by
-		// double-checking that directory doesn't exist.
-		dir, err1 := os.Lstat(path)
-		if err1 == nil && dir.IsDir() {
-			return nil
-		}
-		return err
-	}
-	return nil
-}
-
 func (f *BasicFilesystem) Unhide(name string) error {
 	name, err := f.rooted(name)
 	if err != nil {
diff --git a/lib/fs/fakefs.go b/lib/fs/fakefs.go
index ef0ccd7c5..ff69dfd32 100644
--- a/lib/fs/fakefs.go
+++ b/lib/fs/fakefs.go
@@ -153,7 +153,7 @@ func newFakeFilesystem(rootURI string, _ ...Option) *fakeFS {
 		for (files == 0 || createdFiles < files) && (maxsize == 0 || writtenData>>20 < int64(maxsize)) {
 			dir := filepath.Join(fmt.Sprintf("%02x", rng.Intn(255)), fmt.Sprintf("%02x", rng.Intn(255)))
 			file := fmt.Sprintf("%016x", rng.Int63())
-			_ = fs.MkdirAll(dir, 0o755)
+			_ = fs.MkdirAll(dir, ModePerm)
 
 			fd, _ := fs.Create(filepath.Join(dir, file))
 			createdFiles++
@@ -169,7 +169,7 @@ func newFakeFilesystem(rootURI string, _ ...Option) *fakeFS {
 
 	if !nostfolder {
 		// Also create a default folder marker for good measure
-		_ = fs.Mkdir(".stfolder", 0o700)
+		_ = fs.Mkdir(".stfolder", ModePerm)
 	}
 
 	// We only set the latency after doing the operations required to create
diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go
index c13024f52..b9ea673c4 100644
--- a/lib/model/folder_sendrecv.go
+++ b/lib/model/folder_sendrecv.go
@@ -701,7 +701,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, 0o755); err != nil {
+	if err := f.mtimefs.MkdirAll(parent, fs.ModePerm); err != nil {
 		f.newPullError(file, fmt.Errorf("creating parent dir: %w", err))
 		return false
 	}
diff --git a/lib/model/model.go b/lib/model/model.go
index 4c82a81eb..a57bd23ab 100644
--- a/lib/model/model.go
+++ b/lib/model/model.go
@@ -1784,7 +1784,7 @@ func (m *model) handleAutoAccepts(deviceID protocol.DeviceID, folder protocol.Fo
 
 			// Attempt to create it to make sure it does, now.
 			fullPath := filepath.Join(defaultFolderCfg.Path, path)
-			if err := defaultPathFs.MkdirAll(path, 0o700); err != nil {
+			if err := defaultPathFs.MkdirAll(path, fs.ModePerm); err != nil {
 				slog.Error("Failed to create path for auto-accepted folder", folder.LogAttr(), slogutil.FilePath(fullPath), slogutil.Error(err))
 				continue
 			}
diff --git a/lib/versioner/util.go b/lib/versioner/util.go
index 221de5dc5..dc2e723ad 100644
--- a/lib/versioner/util.go
+++ b/lib/versioner/util.go
@@ -155,7 +155,7 @@ func archiveFile(method fs.CopyRangeMethod, srcFs, dstFs fs.Filesystem, filePath
 	if err != nil {
 		if fs.IsNotExist(err) {
 			slog.Debug("Creating versions dir")
-			err := dstFs.MkdirAll(".", 0o755)
+			err := dstFs.MkdirAll(".", fs.ModePerm)
 			if err != nil {
 				return err
 			}
@@ -328,7 +328,7 @@ func restoreFile(method fs.CopyRangeMethod, src, dst fs.Filesystem, filePath str
 		return err
 	}
 
-	_ = dst.MkdirAll(filepath.Dir(filePath), 0o755)
+	_ = dst.MkdirAll(filepath.Dir(filePath), fs.ModePerm)
 	err := osutil.RenameOrCopy(method, src, dst, sourceFile, filePath)
 	_ = dst.Chtimes(filePath, sourceMtime, sourceMtime)
 	return err
diff --git a/script/authors.go b/script/authors.go
index 23865cc1f..cdb3fa4d1 100644
--- a/script/authors.go
+++ b/script/authors.go
@@ -93,7 +93,7 @@ func main() {
 	}
 	bs = authorsRe.ReplaceAll(bs, []byte("id=\"contributor-list\">\n"+replacement+"\n          </div>"))
 
-	if err := os.WriteFile(htmlFile, bs, 0o644); err != nil {
+	if err := os.WriteFile(htmlFile, bs, 0o666); err != nil {
 		log.Fatal(err)
 	}
 
diff --git a/script/copyrights.go b/script/copyrights.go
index 89a78833e..8c2b4f37e 100644
--- a/script/copyrights.go
+++ b/script/copyrights.go
@@ -265,7 +265,7 @@ func readAll(path string) []byte {
 }
 
 func writeFile(path string, data string) {
-	err := os.WriteFile(path, []byte(data), 0o644)
+	err := os.WriteFile(path, []byte(data), 0o666)
 	if err != nil {
 		log.Fatal(err)
 	}