felixfoertsch/syncthing
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 10 Wiki Activity

fix: let umask do the thing (#10723)

Browse Source 
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>
This commit is contained in:
Jakob Borg
2026-06-03 10:54:04 +02:00
committed by GitHub
parent f93306c819
commit 6df85dc95c
19 changed files with 30 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/api/api.go
+1 -1
View File
@@ -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))
}
lib/config/folderconfiguration.go
+3 -12
View File
@@ -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
lib/config/migrations.go
+6 -14
View File
@@ -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))
lib/fs/basicfs.go
+1 -1
View File
@@ -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) {
lib/fs/basicfs_unix.go
-4
View File
@@ -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 {
lib/fs/basicfs_windows.go
-51
View File
@@ -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 {
lib/fs/fakefs.go
+2 -2
View File
@@ -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
lib/model/folder_sendrecv.go
+1 -1
View File
@@ -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
}
lib/model/model.go
+1 -1
View File
@@ -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
}
lib/versioner/util.go
+2 -2
View File
@@ -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