Merge pull request #4354 from apostasie/2025-06-fixes

Hardening: internal/filesystem: prevent inode change on failure

Author: AkihiroSuda

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
 		}
 	}