diff --git a/src/shell/builtin/rm.zig b/src/shell/builtin/rm.zig index 7a5938f6fb..234baba71e 100644 --- a/src/shell/builtin/rm.zig +++ b/src/shell/builtin/rm.zig @@ -923,7 +923,7 @@ pub const ShellRmTask = struct { } debug("[removeEntryDir] remove after children {s}", .{path}); - switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR)) { + switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR | std.posix.AT.SYMLINK_NOFOLLOW)) { .result => { switch (this.verboseDeleted(dir_task, path)) { .err => |e| return .{ .err = e }, @@ -1083,7 +1083,7 @@ pub const ShellRmTask = struct { } }; const dirfd = this.cwd; - switch (ShellSyscall.unlinkatWithFlags(dirfd, path, 0)) { + switch (ShellSyscall.unlinkatWithFlags(dirfd, path, std.posix.AT.SYMLINK_NOFOLLOW)) { .result => return this.verboseDeleted(parent_dir_task, path), .err => |e| { debug("unlinkatWithFlags({s}) = {s}", .{ path, @tagName(e.getErrno()) }); @@ -1107,7 +1107,7 @@ pub const ShellRmTask = struct { // If `path` points to a directory, then it is deleted (if empty) or we handle it as a directory // If it's actually a file, we get an error so we don't need to call `stat` to check that. if (this.opts.recursive or this.opts.remove_empty_dirs) { - return switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR)) { + return switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR | std.posix.AT.SYMLINK_NOFOLLOW)) { // it was empty, we saved a syscall .result => return this.verboseDeleted(parent_dir_task, path), .err => |e2| { diff --git a/src/sys.zig b/src/sys.zig index 49e93848a3..6685faaa44 100644 --- a/src/sys.zig +++ b/src/sys.zig @@ -2834,9 +2834,11 @@ pub fn unlinkatWithFlags(dirfd: bun.FileDescriptor, to: anytype, flags: c_uint) return unlinkatWithFlags(dirfd, bun.strings.toNTPath(w_buf, bun.span(to)), flags); } + // const follow_symlinks = flags return bun.windows.DeleteFileBun(to, .{ .dir = if (dirfd != bun.invalid_fd) dirfd.cast() else null, .remove_dir = flags & std.posix.AT.REMOVEDIR != 0, + .follow_symlinks = flags & std.posix.AT.SYMLINK_NOFOLLOW == 0, }); } diff --git a/src/windows.zig b/src/windows.zig index c5ca2b1fa5..365d3a1f3e 100644 --- a/src/windows.zig +++ b/src/windows.zig @@ -3483,6 +3483,7 @@ pub extern "kernel32" fn SetConsoleCP(wCodePageID: std.os.windows.UINT) callconv pub const DeleteFileOptions = struct { dir: ?HANDLE, remove_dir: bool = false, + follow_symlinks: bool = true, }; const FILE_DISPOSITION_DELETE: ULONG = 0x00000001; @@ -3493,10 +3494,11 @@ const FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE: ULONG = 0x00000010; // Copy-paste of the standard library function except without unreachable. pub fn DeleteFileBun(sub_path_w: []const u16, options: DeleteFileOptions) bun.JSC.Maybe(void) { + const FOLLOW_SYMLINKS = if (options.follow_symlinks) FILE_OPEN_REPARSE_POINT else 0; const create_options_flags: ULONG = if (options.remove_dir) - FILE_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT + FILE_DIRECTORY_FILE | FOLLOW_SYMLINKS else - windows.FILE_NON_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT; // would we ever want to delete the target instead? + windows.FILE_NON_DIRECTORY_FILE | FOLLOW_SYMLINKS; // would we ever want to delete the target instead? Yes, `rm` is expected to delete symlinks, not the actual files const path_len_bytes = @as(u16, @intCast(sub_path_w.len * 2)); var nt_name = UNICODE_STRING{ diff --git a/test/js/bun/shell/commands/rm.test.ts b/test/js/bun/shell/commands/rm.test.ts index 114b4a9ece..0486d9cf3f 100644 --- a/test/js/bun/shell/commands/rm.test.ts +++ b/test/js/bun/shell/commands/rm.test.ts @@ -144,6 +144,47 @@ foo/ expect(await fileExists(`${tempdir}/sub_dir_files`)).toBeTrue(); } }); + + test("removes symlinks, not the files referenced by the links", async () => { + const tempdir = tempDirWithFiles("rm-symlinks", { + "target.txt": "original content", + "dir/file.txt": "directory file", + }); + + // Create symlinks + await $`ln -s ${tempdir}/target.txt ${tempdir}/link.txt`.cwd(tempdir); + await $`ln -s ${tempdir}/dir ${tempdir}/dirlink`.cwd(tempdir); + + // Verify symlinks exist and point to correct targets + expect(await fileExists(`${tempdir}/link.txt`)).toBeTrue(); + expect(await fileExists(`${tempdir}/dirlink`)).toBeTrue(); + expect(await Bun.file(`${tempdir}/link.txt`).text()).toBe("original content"); + expect(await Bun.file(`${tempdir}/dirlink/file.txt`).text()).toBe("directory file"); + + // Remove the symlinks + { + const { stdout, exitCode } = await $`rm -v ${tempdir}/link.txt`; + expect(stdout.toString()).toEqual(`${tempdir}/link.txt\n`); + expect(exitCode).toBe(0); + } + + { + const { stdout, exitCode } = await $`rm -v ${tempdir}/dirlink`; + expect(stdout.toString()).toEqual(`${tempdir}/dirlink\n`); + expect(exitCode).toBe(0); + } + + // Verify symlinks are gone but targets remain + expect(await fileExists(`${tempdir}/link.txt`)).toBeFalse(); + expect(await fileExists(`${tempdir}/dirlink`)).toBeFalse(); + expect(await fileExists(`${tempdir}/target.txt`)).toBeTrue(); + expect(await fileExists(`${tempdir}/dir`)).toBeTrue(); + expect(await fileExists(`${tempdir}/dir/file.txt`)).toBeTrue(); + + // Verify target files still have their content + expect(await Bun.file(`${tempdir}/target.txt`).text()).toBe("original content"); + expect(await Bun.file(`${tempdir}/dir/file.txt`).text()).toBe("directory file"); + }); }); function packagejson() {