diff --git a/src/runtime/shell/builtin/mv.rs b/src/runtime/shell/builtin/mv.rs index 933837d26c7e..29871b3f17ef 100644 --- a/src/runtime/shell/builtin/mv.rs +++ b/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. diff --git a/src/sys/lib.rs b/src/sys/lib.rs index 1e3ce5de1d25..41c58414be20 100644 --- a/src/sys/lib.rs +++ b/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 { 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 ); diff --git a/test/js/bun/shell/commands/mv.test.ts b/test/js/bun/shell/commands/mv.test.ts index 34b256354708..991f03d5ce10 100644 --- a/test/js/bun/shell/commands/mv.test.ts +++ b/test/js/bun/shell/commands/mv.test.ts @@ -6,7 +6,9 @@ import { chmodSync, constants, existsSync, + lchownSync, lstatSync, + lutimesSync, mkdirSync, readdirSync, readFileSync, @@ -14,6 +16,7 @@ import { 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 {