From 4d994341317c2e700370950bd5fd9ac05963fc66 Mon Sep 17 00:00:00 2001
From: Simon Ruderich <simon@ruderich.org>
Date: Tue, 18 Feb 2020 06:02:17 +0100
Subject: [PATCH] Use github.com/google/renameio for atomic file updates

This also fixes a very unlikely race condition which could delete a
temporary file from a concurrent nsscash run if it chooses the same
temporary file name after os.Rename() was called but before our deferred
os.Remove() remove it. This race would cause the second nsscash run to
fail but not cause any corruption of persistent files.
---
 README.adoc |  3 ++-
 file.go     | 18 ++++--------------
 go.mod      |  1 +
 go.sum      |  2 ++
 main.go     | 15 ++++++---------
 state.go    | 19 +++++--------------
 6 files changed, 20 insertions(+), 38 deletions(-)

diff --git a/README.adoc b/README.adoc
index 93deb40..8821d8a 100644
--- a/README.adoc
+++ b/README.adoc
@@ -68,8 +68,9 @@ Nsscash is licensed under AGPL version 3 or later.
 == REQUIREMENTS
 
 - Go, for `nsscash`
-  * github.com/pkg/errors
   * github.com/BurntSushi/toml
+  * github.com/google/renameio
+  * github.com/pkg/errors
 - C compiler, for `libnss_cash.so.2`
 
 - HTTP(S) server to provide the passwd/group/etc. files
diff --git a/file.go b/file.go
index c7e5809..f382ba4 100644
--- a/file.go
+++ b/file.go
@@ -30,6 +30,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/google/renameio"
 	"github.com/pkg/errors"
 )
 
@@ -164,18 +165,11 @@ func deployFile(file *File) error {
 		return fmt.Errorf("refusing to write empty file")
 	}
 
-	// Write the file in an atomic fashion by creating a temporary file
-	// and renaming it over the target file
-
-	dir := filepath.Dir(file.Path)
-	name := filepath.Base(file.Path)
-
-	f, err := ioutil.TempFile(dir, "tmp-"+name+"-")
+	f, err := renameio.TempFile(filepath.Dir(file.Path), file.Path)
 	if err != nil {
 		return err
 	}
-	defer os.Remove(f.Name())
-	defer f.Close()
+	defer f.Cleanup()
 
 	// Apply permissions/user/group from the target file but remove the
 	// write permissions to discourage manual modifications, use Stat
@@ -204,9 +198,5 @@ func deployFile(file *File) error {
 	if err != nil {
 		return err
 	}
-	err = f.Sync()
-	if err != nil {
-		return err
-	}
-	return os.Rename(f.Name(), file.Path)
+	return f.CloseAtomicallyReplace()
 }
diff --git a/go.mod b/go.mod
index 60a4e0e..46f1415 100644
--- a/go.mod
+++ b/go.mod
@@ -4,5 +4,6 @@ go 1.12
 
 require (
 	github.com/BurntSushi/toml v0.3.1
+	github.com/google/renameio v0.1.1-0.20200217212219-353f81969824
 	github.com/pkg/errors v0.8.1
 )
diff --git a/go.sum b/go.sum
index 69a45f8..9169c08 100644
--- a/go.sum
+++ b/go.sum
@@ -1,4 +1,6 @@
 github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
+github.com/google/renameio v0.1.1-0.20200217212219-353f81969824 h1:9q700G0beHecUuiZOuKgNqNsGQixTeDLnzVZ5nsW3lc=
+github.com/google/renameio v0.1.1-0.20200217212219-353f81969824/go.mod h1:t/HQoYBZSsWSNK35C6CO/TpPLDVWvxOHboWUAweKUpk=
 github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
diff --git a/main.go b/main.go
index 31ef049..5248a77 100644
--- a/main.go
+++ b/main.go
@@ -25,6 +25,8 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+
+	"github.com/google/renameio"
 )
 
 func main() {
@@ -130,29 +132,24 @@ func mainConvert(typ, srcPath, dstPath string) error {
 		return fmt.Errorf("unsupported file type %v", t)
 	}
 
-	dstDir := filepath.Dir(dstPath)
-	dstName := filepath.Base(dstPath)
-
 	// We must create the file first or deployFile() will abort; this is
 	// ugly because deployFile() already performs an atomic replacement
 	// but the simplest solution with the least duplicate code
-	f, err := ioutil.TempFile(dstDir, "tmp-"+dstName+"-")
+	f, err := renameio.TempFile(filepath.Dir(dstPath), dstPath)
 	if err != nil {
 		return err
 	}
-	tmpPath := f.Name()
-	defer os.Remove(tmpPath)
-	f.Close()
+	defer f.Cleanup()
 
 	err = deployFile(&File{
 		Type: t,
 		Url:  srcPath,
-		Path: tmpPath,
+		Path: f.Name(),
 		body: x.Bytes(),
 	})
 	if err != nil {
 		return err
 	}
 
-	return os.Rename(tmpPath, dstPath)
+	return f.CloseAtomicallyReplace()
 }
diff --git a/state.go b/state.go
index 4b31241..43fc598 100644
--- a/state.go
+++ b/state.go
@@ -23,6 +23,8 @@ import (
 	"os"
 	"path/filepath"
 	"time"
+
+	"github.com/google/renameio"
 )
 
 type State struct {
@@ -68,26 +70,15 @@ func WriteState(path string, state *State) error {
 		return err
 	}
 
-	// Write the file in an atomic fashion by creating a temporary file
-	// and renaming it over the target file
-
-	dir := filepath.Dir(path)
-	name := filepath.Base(path)
-
-	f, err := ioutil.TempFile(dir, "tmp-"+name+"-")
+	f, err := renameio.TempFile(filepath.Dir(path), path)
 	if err != nil {
 		return err
 	}
-	defer os.Remove(f.Name())
-	defer f.Close()
+	defer f.Cleanup()
 
 	_, err = f.Write(x)
 	if err != nil {
 		return err
 	}
-	err = f.Sync()
-	if err != nil {
-		return err
-	}
-	return os.Rename(f.Name(), path)
+	return f.CloseAtomicallyReplace()
 }
-- 
2.43.2