From 22b3a73109751928331032868cad9be48ece9c2b Mon Sep 17 00:00:00 2001 From: Esa Kataja Date: Sat, 16 May 2026 20:27:40 +0300 Subject: [PATCH] fix: refuse destination collisions and fall back on EXDEV in mover Centralize all move helpers through a single safeRename function that stats the destination first and refuses to overwrite (bug #14), and falls back to copy+fsync+remove when os.Rename returns EXDEV across mounted filesystems (bug #13). Drops the dead MoveToOutput helper. --- internal/mover/mover.go | 82 ++++++++++++++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 13 deletions(-) diff --git a/internal/mover/mover.go b/internal/mover/mover.go index ff0cac0..825dcec 100644 --- a/internal/mover/mover.go +++ b/internal/mover/mover.go @@ -1,36 +1,92 @@ package mover import ( + "errors" "fmt" + "io" "os" "path/filepath" + "syscall" ) -func MoveToOutput(input, outputDir string) error { - if err := os.MkdirAll(outputDir, 0755); err != nil { - return fmt.Errorf("creating output dir: %w", err) +// safeRename moves src to dst. Refuses to overwrite an existing dst. +// Falls back to copy+remove on EXDEV (cross-device) errors. +func safeRename(src, dst string) error { + if _, err := os.Stat(dst); err == nil { + return fmt.Errorf("destination %s already exists", dst) + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("stat destination %s: %w", dst, err) } - filename := filepath.Base(input) - dest := filepath.Join(outputDir, filename) - return os.Rename(input, dest) + + err := os.Rename(src, dst) + if err == nil { + return nil + } + + var linkErr *os.LinkError + if !errors.As(err, &linkErr) || !errors.Is(err, syscall.EXDEV) { + return fmt.Errorf("renaming %s to %s: %w", src, dst, err) + } + + return copyAndRemove(src, dst) +} + +// copyAndRemove implements the cross-device fallback: copy src to dst, fsync, +// then remove src. Uses O_EXCL to belt-and-suspenders against a race. +func copyAndRemove(src, dst string) error { + in, err := os.Open(src) + if err != nil { + return fmt.Errorf("opening source %s: %w", src, err) + } + defer in.Close() + + out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0644) + if err != nil { + return fmt.Errorf("creating destination %s: %w", dst, err) + } + + if _, err := io.Copy(out, in); err != nil { + out.Close() + os.Remove(dst) + return fmt.Errorf("copying %s to %s: %w", src, dst, err) + } + + if err := out.Sync(); err != nil { + out.Close() + os.Remove(dst) + return fmt.Errorf("syncing destination %s: %w", dst, err) + } + + if err := out.Close(); err != nil { + os.Remove(dst) + return fmt.Errorf("closing destination %s: %w", dst, err) + } + + if err := in.Close(); err != nil { + return fmt.Errorf("closing source %s: %w", src, err) + } + + if err := os.Remove(src); err != nil { + return fmt.Errorf("removing source %s: %w", src, err) + } + + return nil } func MoveToFailed(input, failedDir string) error { if err := os.MkdirAll(failedDir, 0755); err != nil { return fmt.Errorf("creating failed dir: %w", err) } - filename := filepath.Base(input) - dest := filepath.Join(failedDir, filename) - return os.Rename(input, dest) + dest := filepath.Join(failedDir, filepath.Base(input)) + return safeRename(input, dest) } func MoveToOriginals(input, originalsDir string) error { if err := os.MkdirAll(originalsDir, 0755); err != nil { return fmt.Errorf("creating originals dir: %w", err) } - filename := filepath.Base(input) - dest := filepath.Join(originalsDir, filename) - return os.Rename(input, dest) + dest := filepath.Join(originalsDir, filepath.Base(input)) + return safeRename(input, dest) } func Delete(path string) error { @@ -38,5 +94,5 @@ func Delete(path string) error { } func Rename(source, dest string) error { - return os.Rename(source, dest) + return safeRename(source, dest) }