From dc7971fb4387d383534739a67ac00aaeb454935a Mon Sep 17 00:00:00 2001 From: apostasie Date: Mon, 23 Jun 2025 11:11:44 -0700 Subject: [PATCH] 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 --- pkg/internal/filesystem/helpers.go | 18 +++++++++--------- pkg/internal/filesystem/writefile_rollback.go | 9 ++++++++- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/pkg/internal/filesystem/helpers.go b/pkg/internal/filesystem/helpers.go index f9be0cb2..75ce3710 100644 --- a/pkg/internal/filesystem/helpers.go +++ b/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 diff --git a/pkg/internal/filesystem/writefile_rollback.go b/pkg/internal/filesystem/writefile_rollback.go index 31ec35c3..46085264 100644 --- a/pkg/internal/filesystem/writefile_rollback.go +++ b/pkg/internal/filesystem/writefile_rollback.go @@ -61,6 +61,11 @@ 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 @@ -68,8 +73,10 @@ func WriteFileWithRollback(filename string, data []byte, perm os.FileMode) (roll // 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 } }