From 6cd8fcd9d85f803060c6fb0c424930117841efbe Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Sat, 30 May 2026 02:32:28 +0200
Subject: [PATCH] libutil: handle all rename errors in moveFile

what the actual fuck. swallowing all errors *except* EXDEV is not okay.
renames do not do this, so moves should not do it either. luckily we do
not use moveFile anywhere except the store path registration code. this
may or may not have caused problems in the past. probably nobody knows.

Change-Id: I2b0255a5703983cbd129abc3219c11ac7171fd12
---
 lix/libutil/file-system.cc  | 23 ++++++++++++-----------
 tests/unit/libutil/tests.cc | 26 ++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/lix/libutil/file-system.cc b/lix/libutil/file-system.cc
index 0e688ec20..8702968a7 100644
--- a/lix/libutil/file-system.cc
+++ b/lix/libutil/file-system.cc
@@ -824,22 +824,23 @@ void moveFile(const Path & oldName, const Path & newName)
     try {
         fs::rename(oldName, newName);
     } catch (fs::filesystem_error & e) { // NOLINT(lix-foreign-exceptions)
-        auto oldPath = fs::path(oldName);
-        auto newPath = fs::path(newName);
-        // For the move to be as atomic as possible, copy to a temporary
-        // directory
         try {
+            if (e.code().value() != EXDEV) {
+                throw SysError(e.code().value(), "failed to move %s to %s", oldName, newName);
+            }
+            auto oldPath = fs::path(oldName);
+            auto newPath = fs::path(newName);
+            // For the move to be as atomic as possible, copy to a temporary
+            // directory
             fs::path temp = createTempSubdir(newPath.parent_path(), "rename-tmp");
             Finally removeTemp = [&]() { fs::remove(temp); };
             auto tempCopyTarget = temp / "copy-target";
-            if (e.code().value() == EXDEV) {
-                fs::remove(newPath);
-                printTaggedWarning("Can’t rename %s as %s, copying instead", oldName, newName);
-                copy(fs::directory_entry(oldPath), tempCopyTarget, {.deleteAfter = true});
-                fs::rename(tempCopyTarget, newPath);
-            }
+            fs::remove(newPath);
+            printTaggedWarning("Can’t rename %s as %s, copying instead", oldName, newName);
+            copy(fs::directory_entry(oldPath), tempCopyTarget, {.deleteAfter = true});
+            fs::rename(tempCopyTarget, newPath);
         } catch (fs::filesystem_error & e) { // NOLINT(lix-foreign-exceptions)
-            throw Error("failed to move %s to %s: %s", oldName, newName, e.what());
+            throw SysError(e.code().value(), "failed to move %s to %s", oldName, newName);
         }
     }
 }
diff --git a/tests/unit/libutil/tests.cc b/tests/unit/libutil/tests.cc
index 34ced9f18..064d03b56 100644
--- a/tests/unit/libutil/tests.cc
+++ b/tests/unit/libutil/tests.cc
@@ -1,3 +1,4 @@
+#include "lix/libstore/temporary-dir.hh"
 #include "lix/libutil/c-calls.hh"
 #include "lix/libutil/file-system.hh"
 #include "lix/libutil/processes.hh"
@@ -210,6 +211,31 @@ namespace nix {
         ASSERT_FALSE(pathExists("/schnitzel/darmstadt/pommes"));
     }
 
+    /* ----------------------------------------------------------------------------
+     * moveFile
+     * --------------------------------------------------------------------------*/
+
+    TEST(moveFile, handlesErrors)
+    {
+        auto tmpDir = createTempDir();
+        AutoDelete _delete(tmpDir);
+
+        auto source = tmpDir + "/source";
+        auto target = tmpDir + "/target";
+
+        // xdev should cause copies, everything else must throw. this is a small selection.
+        ASSERT_THROW(moveFile(source, target), Error); // ENOENT
+        createDirs(source);
+        createSymlink(target, target);
+        ASSERT_THROW(moveFile(source, target), Error); // ENOTDIR
+        ASSERT_THROW(moveFile(target, source), Error); // EISDIR
+        ASSERT_THROW(moveFile(target + "/foo", source), Error); // ELOOP
+
+        deletePath(target);
+        createDirs(target + "/snafu");
+        ASSERT_THROW(moveFile(source, target), Error); // ENOTEMPTY or EEXIST
+    }
+
     /* ----------------------------------------------------------------------------
      * AutoCloseFD::guessOrInventPath
      * --------------------------------------------------------------------------*/