internal/filesystem minor fix

In case of error during write, the destination is being removed (before being possibly restored).
This may lead to certain (failure) scenarios where the inode would change, effectively breaking
container mounted files.

Signed-off-by: apostasie <[email protected]>

Author: apostasie

pkg/internal/filesystem/helpers.go

@@ -54,6 +54,7 @@ func ensureRecovery(filename string) (err error) {
 		if err = backupRestore(filename); err != nil {
 			return err
 		}
+		_ = backupRemove(filename)
 	} else {
 		// We do not see a backup.
 		// Do we have a final destination then?
@@ -101,6 +102,10 @@ func backupRestore(path string) error {
 	return err
 }
 
+func backupRemove(path string) error {
+	return os.Remove(backupLocation(path))
+}
+
 // backupExists checks if a backup file exists for file located at `path`.
 func backupExists(path string) (bool, error) {
 	_, err := os.Stat(backupLocation(path))
@@ -190,16 +195,16 @@ func internalCopy(sourcePath, destinationPath string) (err error) {
 		return err
 	}
 
+	defer func() {
+		err = errors.Join(err, source.Close())
+	}()
+
 	// Read file length
 	srcInfo, err := source.Stat()
 	if err != nil {
 		return err
 	}
 
-	defer func() {
-		err = errors.Join(err, source.Close())
-	}()
-
 	return fileWrite(source, srcInfo.Size(), destinationPath, privateFilePermission, srcInfo.ModTime())
 }
 
@@ -220,11 +225,6 @@ func fileWrite(source io.Reader, size int64, destinationPath string, perm os.Fil
 		if mustClose {
 			err = errors.Join(err, destination.Close())
 		}
-
-		// Remove destination if we failed anywhere. Ignore removal failures.
-		if err != nil {
-			_ = os.Remove(destinationPath)
-		}
 	}()
 
 	// Copy over

pkg/internal/filesystem/writefile_rollback.go

@@ -61,15 +61,22 @@ func WriteFileWithRollback(filename string, data []byte, perm os.FileMode) (roll
 		}
 	}
 
+	// Make sure no leftover backup file is here
+	// Note: this happens after a successful write. Generally not a problem, except if the file is then deleted,
+	// then written to again, and that second write would fail.
+	_ = backupRemove(filename)
+
 	// Create the marker. Failure to do so is a hard error.
 	if err = markerCreate(filename, markerData); err != nil {
 		return nil, err
 	}
 
 	// If the file exists, we need to back it up.
 	if markerData == "" {
-		// Back it up now.
+		// Back it up now. Remove on failure.
 		if err = backupSave(filename); err != nil {
+			_ = backupRemove(filename)
+			_ = markerRemove(filename)
 			return nil, err
 		}
 	}