From 5ec312ca0683c0d4700a832ff6483dde9b1a98c0 Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Wed, 22 Sep 2021 22:28:57 +0200
Subject: [PATCH] sftp: Implement atomic uploads

Create a temporary file with a sufficiently random name to essentially
avoid any chance of conflicts. Once the upload has finished remove the
temporary suffix. Interrupted upload thus will be ignored by restic.
---
 changelog/unreleased/issue-3003 | 10 ++++++++++
 internal/backend/sftp/sftp.go   | 27 +++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 4 deletions(-)
 create mode 100644 changelog/unreleased/issue-3003

diff --git a/changelog/unreleased/issue-3003 b/changelog/unreleased/issue-3003
new file mode 100644
index 000000000..6e865d600
--- /dev/null
+++ b/changelog/unreleased/issue-3003
@@ -0,0 +1,10 @@
+Enhancement: Atomic uploads for SFTP
+
+The SFTP backend did not upload files atomically. An interrupted upload could
+leave an incomplete file behind which could prevent restic from accessing the
+repository.
+
+Uploads in the SFTP backend are now done atomically.
+
+https://github.com/restic/restic/issues/3003
+https://github.com/restic/restic/pull/3524
diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go
index 110239277..3e803a0f4 100644
--- a/internal/backend/sftp/sftp.go
+++ b/internal/backend/sftp/sftp.go
@@ -3,6 +3,8 @@ package sftp
 import (
 	"bufio"
 	"context"
+	"crypto/rand"
+	"encoding/hex"
 	"fmt"
 	"hash"
 	"io"
@@ -252,6 +254,17 @@ func Join(parts ...string) string {
 	return path.Clean(path.Join(parts...))
 }
 
+// tempSuffix generates a random string suffix that should be sufficiently long
+// to avoid accidential conflicts
+func tempSuffix() string {
+	var nonce [16]byte
+	_, err := rand.Read(nonce[:])
+	if err != nil {
+		panic(err)
+	}
+	return hex.EncodeToString(nonce[:])
+}
+
 // Save stores data in the backend at the handle.
 func (r *SFTP) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
 	debug.Log("Save %v", h)
@@ -264,10 +277,11 @@ func (r *SFTP) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader
 	}
 
 	filename := r.Filename(h)
+	tmpFilename := filename + "-restic-temp-" + tempSuffix()
 	dirname := r.Dirname(h)
 
 	// create new file
-	f, err := r.c.OpenFile(filename, os.O_CREATE|os.O_EXCL|os.O_WRONLY)
+	f, err := r.c.OpenFile(tmpFilename, os.O_CREATE|os.O_EXCL|os.O_WRONLY)
 
 	if r.IsNotExist(err) {
 		// error is caused by a missing directory, try to create it
@@ -276,7 +290,7 @@ func (r *SFTP) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader
 			debug.Log("error creating dir %v: %v", r.Dirname(h), mkdirErr)
 		} else {
 			// try again
-			f, err = r.c.OpenFile(filename, os.O_CREATE|os.O_EXCL|os.O_WRONLY)
+			f, err = r.c.OpenFile(tmpFilename, os.O_CREATE|os.O_EXCL|os.O_WRONLY)
 		}
 	}
 
@@ -298,7 +312,7 @@ func (r *SFTP) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader
 		rmErr := r.c.Remove(f.Name())
 		if rmErr != nil {
 			debug.Log("sftp: failed to remove broken file %v: %v",
-				filename, rmErr)
+				f.Name(), rmErr)
 		}
 
 		err = r.checkNoSpace(dirname, rd.Length(), err)
@@ -318,7 +332,12 @@ func (r *SFTP) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader
 	}
 
 	err = f.Close()
-	return errors.Wrap(err, "Close")
+	if err != nil {
+		return errors.Wrap(err, "Close")
+	}
+
+	err = r.c.Rename(tmpFilename, filename)
+	return errors.Wrap(err, "Rename")
 }
 
 // checkNoSpace checks if err was likely caused by lack of available space