From 970e50d1f1d5bc49a3bf0697e82b2472fc314714 Mon Sep 17 00:00:00 2001
From: Vilbrekin <vilbrekin@gmail.com>
Date: Sun, 26 Oct 2014 01:54:50 +0200
Subject: [PATCH] Correctly check whether parent directory is writable for
 current user. "&04" was checking if file is readable by others, while "&0200"
 checks if it's writable for current user.

(Fixes #904)
---
 CONTRIBUTORS                             |  1 +
 internal/model/sharedpullerstate.go      |  6 ++++--
 internal/model/sharedpullerstate_test.go | 26 +++++++++++++++++++++++-
 internal/model/testdata/read_only_dir/a  |  0
 internal/osutil/osutil.go                |  2 +-
 5 files changed, 31 insertions(+), 4 deletions(-)
 create mode 100644 internal/model/testdata/read_only_dir/a

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index af9ec2fb1..9eb21bfe2 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -22,3 +22,4 @@ Phill Luby <phill.luby@newredo.com>
 Ryan Sullivan <kayoticsully@gmail.com>
 Tully Robinson <tully@tojr.org>
 Veeti Paananen <veeti.paananen@rojekti.fi>
+Vil Brekin <vilbrekin@gmail.com>
diff --git a/internal/model/sharedpullerstate.go b/internal/model/sharedpullerstate.go
index 9557b0b67..eeb22a61c 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 b669218c9..8d2f6c181 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 000000000..e69de29bb
diff --git a/internal/osutil/osutil.go b/internal/osutil/osutil.go
index a760a9e8e..66dfd4f91 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.