shell mv: keep timestamps and the symlink owner on the cross-device fallback

src/runtime/shell/builtin/mv.rs

@@ -507,7 +507,7 @@ impl ShellMvBatchedTask {
     ) -> Result<(), bun_sys::Error> {
         use bun_sys::{Dir, E, File, O, S, Tag};
 
-        // The copy belongs to the mover until `copy_owner_and_mode` runs.
+        // The copy belongs to the mover until `copy_metadata` runs.
         const OWNER_ONLY: bun_core::Mode = 0o700;
 
         let st = bun_sys::lstatat(src_dir, src)?;
@@ -530,6 +530,19 @@ impl ShellMvBatchedTask {
             buf[n] = 0;
             let _ = bun_sys::unlinkat(dst_dir, dst);
             bun_sys::symlinkat(ZStr::from_buf(&buf[..], n), dst_dir, dst)?;
+            #[cfg(unix)]
+            {
+                // Same order as `copy_metadata`: times, then owner.
+                let (atime, mtime) = Self::source_times(&st);
+                let _ = bun_sys::utimensat(dst_dir, dst, atime, mtime, libc::AT_SYMLINK_NOFOLLOW);
+                let _ = bun_sys::fchownat(
+                    dst_dir,
+                    dst,
+                    st.st_uid as _,
+                    st.st_gid as _,
+                    libc::AT_SYMLINK_NOFOLLOW,
+                );
+            }
             return bun_sys::unlinkat(src_dir, src);
         }
 
@@ -575,8 +588,9 @@ impl ShellMvBatchedTask {
                 let name_z = ZStr::from_buf(&nbuf[..], name.len());
                 Self::move_across_devices(sd.fd(), name_z, dd.fd(), name_z)?;
             }
+            // After the children: each child copy bumps the mtime of the new directory.
             #[cfg(unix)]
-            Self::copy_owner_and_mode(dd.fd(), &st);
+            Self::copy_metadata(dd.fd(), &st);
             #[cfg(windows)]
             let _ = bun_sys::fchmod(dd.fd(), st.st_mode as bun_core::Mode & 0o7777);
             drop((sd, dd));
@@ -615,14 +629,18 @@ impl ShellMvBatchedTask {
             return Err(e);
         }
         #[cfg(unix)]
-        Self::copy_owner_and_mode(out.fd(), &st);
+        Self::copy_metadata(out.fd(), &st);
         drop((in_, out));
         bun_sys::unlinkat(src_dir, src)
     }
 
+    /// Gives the copy the times, owner and mode of the source, best effort.
     /// POSIX `mv`: the copy gets set-uid and set-gid only if it also gets the owner.
     #[cfg(unix)]
-    fn copy_owner_and_mode(fd: bun_sys::Fd, st: &bun_sys::Stat) {
+    fn copy_metadata(fd: bun_sys::Fd, st: &bun_sys::Stat) {
+        // Times before owner: once the copy belongs to another user, the mover may not set them.
+        let (atime, mtime) = Self::source_times(st);
+        let _ = bun_sys::futimens(fd, atime, mtime);
         let mut mode = st.st_mode as bun_core::Mode & 0o7777;
         // `fchown` first: Linux clears S_ISUID/S_ISGID on chown.
         if bun_sys::fchown(fd, st.st_uid as _, st.st_gid as _).is_err() {
@@ -631,6 +649,19 @@ impl ShellMvBatchedTask {
         let _ = bun_sys::fchmod(fd, mode);
     }
 
+    /// `(atime, mtime)` of the source, for the copy made by [`Self::move_across_devices`].
+    #[cfg(unix)]
+    fn source_times(st: &bun_sys::Stat) -> (bun_sys::TimeLike, bun_sys::TimeLike) {
+        let to_time_like = |t: bun_core::Timespec| bun_sys::TimeLike {
+            sec: t.sec,
+            nsec: t.nsec,
+        };
+        (
+            to_time_like(bun_sys::stat_atime(st)),
+            to_time_like(bun_sys::stat_mtime(st)),
+        )
+    }
+
     /// `renameat(cwd, src, target_fd, basename(src))`. A free fn over the
     /// fields it touches so `src` can borrow `self.sources[_]` while `self.err`
     /// is written by the caller.

src/sys/lib.rs

@@ -2838,6 +2838,18 @@ mod posix_impl {
         );
         Ok(())
     }
+    /// `flags` is `0` or `AT_SYMLINK_NOFOLLOW`.
+    pub fn fchownat(dir: impl AsFd, path: &ZStr, uid: u32, gid: u32, flags: i32) -> Maybe<()> {
+        let dir = dir.as_fd();
+        check_p!(
+            // SAFETY: `dir` is a live fd (or AT_FDCWD); `ZStr::as_ptr()` is a
+            // valid NUL-terminated C string.
+            unsafe { libc::fchownat(dir.native(), path.as_ptr(), uid, gid, flags) },
+            Tag::fchownat,
+            path
+        );
+        Ok(())
+    }
     pub fn fstatat(fd: impl AsFd, path: &ZStr) -> Maybe<Stat> {
         let fd = fd.as_fd();
         let dirfd = if fd.is_valid() {
@@ -2899,18 +2911,23 @@ mod posix_impl {
         Ok(())
     }
     pub fn lutimens(path: &ZStr, atime: TimeLike, mtime: TimeLike) -> Maybe<()> {
+        utimensat(Fd::cwd(), path, atime, mtime, libc::AT_SYMLINK_NOFOLLOW)
+    }
+    /// `flags` is `0` or `AT_SYMLINK_NOFOLLOW`.
+    pub fn utimensat(
+        dir: impl AsFd,
+        path: &ZStr,
+        atime: TimeLike,
+        mtime: TimeLike,
+        flags: i32,
+    ) -> Maybe<()> {
+        let dir = dir.as_fd();
         let ts = [atime.to_timespec(), mtime.to_timespec()];
         check_p!(
-            // SAFETY: `path` is NUL-terminated (`ZStr`); `ts` is a 2-element
-            // stack array and `utimensat` reads exactly two `timespec`s.
-            unsafe {
-                libc::utimensat(
-                    libc::AT_FDCWD,
-                    path.as_ptr(),
-                    ts.as_ptr(),
-                    libc::AT_SYMLINK_NOFOLLOW,
-                )
-            },
+            // SAFETY: `dir` is a live fd (or AT_FDCWD); `path` is NUL-terminated
+            // (`ZStr`); `ts` is a 2-element stack array and `utimensat` reads
+            // exactly two `timespec`s.
+            unsafe { libc::utimensat(dir.native(), path.as_ptr(), ts.as_ptr(), flags) },
             Tag::utimensat,
             path
         );

test/js/bun/shell/commands/mv.test.ts

@@ -6,14 +6,17 @@ import {
   chmodSync,
   constants,
   existsSync,
+  lchownSync,
   lstatSync,
+  lutimesSync,
   mkdirSync,
   readdirSync,
   readFileSync,
   readlinkSync,
   rmSync,
   statSync,
   symlinkSync,
+  utimesSync,
   writeFileSync,
   type Stats,
 } from "node:fs";
@@ -218,6 +221,61 @@ describe("mv", async () => {
       },
     );
 
+    test.skipIf(skip)("keeps atime and mtime across devices", async () => {
+      const [src, dst] = crossDevicePair("times");
+      try {
+        const atime = new Date("2001-02-03T04:05:06Z");
+        const mtime = new Date("2002-03-04T05:06:07Z");
+        const srcFile = join(src, "f.txt");
+        const srcDir = join(src, "d");
+        const srcLink = join(src, "link");
+        writeFileSync(srcFile, "payload\n");
+        mkdirSync(srcDir);
+        writeFileSync(join(srcDir, "child.txt"), "child\n");
+        symlinkSync("does-not-exist", srcLink);
+        utimesSync(srcFile, atime, mtime);
+        utimesSync(join(srcDir, "child.txt"), atime, mtime);
+        utimesSync(srcDir, atime, mtime);
+        lutimesSync(srcLink, atime, mtime);
+
+        const r = await $`mv ${srcFile} ${srcDir} ${srcLink} ${dst}`.quiet();
+        expect(r.stderr.toString()).toBe("");
+        expect(r.exitCode).toBe(0);
+        const times = (p: string) => {
+          const st = lstatSync(p);
+          return { atime: st.atime.toISOString(), mtime: st.mtime.toISOString() };
+        };
+        const expected = { atime: atime.toISOString(), mtime: mtime.toISOString() };
+        expect({
+          file: times(join(dst, "f.txt")),
+          dir: times(join(dst, "d")),
+          child: times(join(dst, "d", "child.txt")),
+          link: times(join(dst, "link")),
+        }).toEqual({ file: expected, dir: expected, child: expected, link: expected });
+      } finally {
+        rmSync(src, { recursive: true, force: true });
+        rmSync(dst, { recursive: true, force: true });
+      }
+    });
+
+    test.skipIf(skip || !isRoot)("keeps the owner of a symlink across devices", async () => {
+      const [src, dst] = crossDevicePair("link-owner");
+      try {
+        const srcLink = join(src, "link");
+        symlinkSync("does-not-exist", srcLink);
+        lchownSync(srcLink, 65534, 65534);
+
+        const r = await $`mv ${srcLink} ${dst}`.quiet();
+        expect(r.stderr.toString()).toBe("");
+        expect(r.exitCode).toBe(0);
+        const st = lstatSync(join(dst, "link"));
+        expect({ uid: st.uid, gid: st.gid }).toEqual({ uid: 65534, gid: 65534 });
+      } finally {
+        rmSync(src, { recursive: true, force: true });
+        rmSync(dst, { recursive: true, force: true });
+      }
+    });
+
     test.skipIf(skip)("FIFO across devices fails fast", async () => {
       const [src, dst] = crossDevicePair("fifo");
       try {