diff --git a/CONTRIBUTORS b/CONTRIBUTORS index af9ec2fb..9eb21bfe 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -22,3 +22,4 @@ Phill Luby Ryan Sullivan Tully Robinson Veeti Paananen +Vil Brekin diff --git a/internal/model/sharedpullerstate.go b/internal/model/sharedpullerstate.go index 9557b0b6..eeb22a61 100644 --- a/internal/model/sharedpullerstate.go +++ b/internal/model/sharedpullerstate.go @@ -20,6 +20,7 @@ import ( "path/filepath" "sync" + "github.com/syncthing/syncthing/internal/osutil" "github.com/syncthing/syncthing/internal/protocol" ) @@ -68,7 +69,7 @@ func (s *sharedPullerState) tempFile() (*os.File, error) { if info, err := os.Stat(dir); err != nil { s.earlyCloseLocked("dst stat dir", err) return nil, err - } else if info.Mode()&04 == 0 { + } else if info.Mode()&0200 == 0 { err := os.Chmod(dir, 0755) if err == nil { defer func() { @@ -136,7 +137,8 @@ func (s *sharedPullerState) earlyCloseLocked(context string, err error) { s.err = err if s.fd != nil { s.fd.Close() - os.Remove(s.tempName) + // Delete temporary file, even if parent dir is read-only + osutil.InWritableDir(func(string) error { os.Remove(s.tempName); return nil }, s.tempName) } s.closed = true } diff --git a/internal/model/sharedpullerstate_test.go b/internal/model/sharedpullerstate_test.go index b669218c..8d2f6c18 100644 --- a/internal/model/sharedpullerstate_test.go +++ b/internal/model/sharedpullerstate_test.go @@ -15,7 +15,11 @@ package model -import "testing" +import ( + "os" + "path/filepath" + "testing" +) func TestSourceFileOK(t *testing.T) { s := sharedPullerState{ @@ -61,3 +65,23 @@ func TestSourceFileBad(t *testing.T) { t.Fatal("Unexpected nil failed()") } } + +// Test creating temporary file inside read-only directory +func TestReadOnlyDir(t *testing.T) { + s := sharedPullerState{ + tempName: "testdata/read_only_dir/.temp_name", + } + + // Ensure dir is read-only (git doesn't store full permissions) + os.Chmod(filepath.Dir(s.tempName), 0555) + + fd, err := s.tempFile() + if err != nil { + t.Fatal(err) + } + if fd == nil { + t.Fatal("Unexpected nil fd") + } + + s.earlyClose("Test done", nil) +} diff --git a/internal/model/testdata/read_only_dir/a b/internal/model/testdata/read_only_dir/a new file mode 100644 index 00000000..e69de29b diff --git a/internal/osutil/osutil.go b/internal/osutil/osutil.go index a760a9e8..66dfd4f9 100644 --- a/internal/osutil/osutil.go +++ b/internal/osutil/osutil.go @@ -66,7 +66,7 @@ func Rename(from, to string) error { // containing `path` is writable for the duration of the call. func InWritableDir(fn func(string) error, path string) error { dir := filepath.Dir(path) - if info, err := os.Stat(dir); err == nil && info.IsDir() && info.Mode()&04 == 0 { + if info, err := os.Stat(dir); err == nil && info.IsDir() && info.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.