diff --git a/src/runtime/shell/builtin/cp.rs b/src/runtime/shell/builtin/cp.rs index 2c2c8b25644e..80da25ae029b 100644 --- a/src/runtime/shell/builtin/cp.rs +++ b/src/runtime/shell/builtin/cp.rs @@ -4,7 +4,7 @@ use crate::node::PathLike; use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind}; use crate::shell::interpreter::{ EventLoopHandle, FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, - ParseFlagResult, ShellTask, parse_flags, unsupported_flag, + ParseFlagResult, ShellTask, parse_flags, reject_empty_path, unsupported_flag, }; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -607,6 +607,12 @@ impl ShellCpTask { ) -> Option { use resolve_path::{Platform, platform}; + if let Err(e) = reject_empty_path(&self.src, bun_sys::Tag::copyfile) + .and_then(|()| reject_empty_path(&self.tgt, bun_sys::Tag::copyfile)) + { + return Some(ShellErr::new_sys(&e)); + } + let mut buf2 = bun_paths::path_buffer_pool::get(); let mut buf3 = bun_paths::path_buffer_pool::get(); // We have to give an absolute path to our cp implementation for it to diff --git a/src/runtime/shell/builtin/mkdir.rs b/src/runtime/shell/builtin/mkdir.rs index a50d8b9aca49..c0d0b53654a1 100644 --- a/src/runtime/shell/builtin/mkdir.rs +++ b/src/runtime/shell/builtin/mkdir.rs @@ -4,7 +4,7 @@ use crate::shell::ExitCode; use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind}; use crate::shell::interpreter::{ EventLoopHandle, FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, - ParseFlagResult, ShellTask, parse_flags, unsupported_flag, + ParseFlagResult, ShellTask, parse_flags, reject_empty_path, unsupported_flag, }; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -296,6 +296,10 @@ impl ShellMkdirTask { fn run_from_thread_pool(this: &mut ShellMkdirTask) { use bun_paths::{Platform, platform, resolve_path}; + if let Err(e) = reject_empty_path(&this.filepath, bun_sys::Tag::mkdir) { + this.err = Some(e); + return; + } // We have to give an absolute path to our mkdir implementation for it // to work with cwd. let mut spill = Vec::new(); diff --git a/src/runtime/shell/builtin/mv.rs b/src/runtime/shell/builtin/mv.rs index 2d143b92a882..288bc74803f7 100644 --- a/src/runtime/shell/builtin/mv.rs +++ b/src/runtime/shell/builtin/mv.rs @@ -6,7 +6,9 @@ use bun_ptr::BackRef; use crate::shell::ExitCode; use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind}; -use crate::shell::interpreter::{Interpreter, NodeId, ShellTask, closefd, shell_openat}; +use crate::shell::interpreter::{ + Interpreter, NodeId, ShellTask, closefd, reject_empty_path, shell_openat, +}; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -484,6 +486,8 @@ impl ShellMvBatchedTask { dst_dir: bun_sys::Fd, dst: &ZStr, ) -> Result<(), bun_sys::Error> { + reject_empty_path(src.as_bytes(), bun_sys::Tag::rename)?; + reject_empty_path(dst.as_bytes(), bun_sys::Tag::rename)?; match bun_sys::renameat(src_dir, src, dst_dir, dst) { Err(e) if e.get_errno() == bun_sys::E::EXDEV => { Self::move_across_devices(src_dir, src, dst_dir, dst).map_err(|e| { diff --git a/src/runtime/shell/builtin/rm.rs b/src/runtime/shell/builtin/rm.rs index b8d5421f3097..db4074c257ee 100644 --- a/src/runtime/shell/builtin/rm.rs +++ b/src/runtime/shell/builtin/rm.rs @@ -7,7 +7,7 @@ use bun_sys::{E, FdExt, dir_iterator}; use crate::shell::ExitCode; use crate::shell::builtin::{Builtin, IoKind, Kind}; use crate::shell::interpreter::{ - EventLoopHandle, Interpreter, NodeId, ShellTask, WorkPoolTask, shell_openat, + EventLoopHandle, Interpreter, NodeId, ShellTask, WorkPoolTask, reject_empty_path, shell_openat, }; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -181,6 +181,10 @@ impl Rm { for i in args_start..argc { let path = Builtin::of(interp, cmd).arg_bytes(i); + // Joined below, `""` would resolve to the cwd itself. + if path.is_empty() { + continue; + } let resolved: &[u8] = if Platform::AUTO.is_absolute(path) { path } else { @@ -1221,7 +1225,9 @@ impl ShellRmTask { vtable: &mut V, ) -> bun_sys::Maybe<()> { let dirfd = self.cwd; - match bun_sys::unlinkat_with_flags(dirfd, path, 0) { + match reject_empty_path(path.as_bytes(), bun_sys::Tag::unlink) + .and_then(|()| bun_sys::unlinkat_with_flags(dirfd, path, 0)) + { Ok(()) => self.verbose_deleted(parent_dir_task, path.as_bytes()), Err(e) => match e.get_errno() { E::ENOENT => { diff --git a/src/runtime/shell/builtin/touch.rs b/src/runtime/shell/builtin/touch.rs index ca306ceea23d..2220f4898563 100644 --- a/src/runtime/shell/builtin/touch.rs +++ b/src/runtime/shell/builtin/touch.rs @@ -2,7 +2,7 @@ use crate::shell::ExitCode; use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind}; use crate::shell::interpreter::{ EventLoopHandle, FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, - ParseFlagResult, ShellTask, parse_flags, unsupported_flag, + ParseFlagResult, ShellTask, parse_flags, reject_empty_path, unsupported_flag, }; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -265,6 +265,10 @@ impl ShellTouchTask { pub(crate) fn run_from_thread_pool(this: &mut ShellTouchTask) { use bun_paths::resolve_path::{self, Platform, platform}; use bun_sys::FdExt as _; + if let Err(e) = reject_empty_path(&this.filepath, bun_sys::Tag::utime) { + this.err = Some(e); + return; + } // We have to give an absolute path. An operand that does not fit the // path buffer is still passed on whole, so the OS reports ENAMETOOLONG // for it like for any other operand. diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index d1735dad6f74..cd68aa7534cc 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -2416,6 +2416,14 @@ pub(crate) fn shell_lstatat(dir: Fd, path_: &bun_core::ZStr) -> bun_sys::Result< } } +/// Resolved by the shell (cwd join, or the Windows `*at()` emulation), `""` would name the cwd. +pub(crate) fn reject_empty_path(path: &[u8], syscall: bun_sys::Tag) -> bun_sys::Result<()> { + if path.is_empty() { + return Err(bun_sys::Error::from_code(bun_sys::E::ENOENT, syscall)); + } + Ok(()) +} + /// POSIX: `bun_sys::openat` with the error tagged `.with_path(path)`. /// Windows: for `O_DIRECTORY` opens, rewrite POSIX-absolute paths via /// `shell_get_path` and use `openDirAtWindowsA(.iterable=true)` + @@ -2427,6 +2435,7 @@ pub(crate) fn shell_openat( flags: i32, perm: bun_sys::Mode, ) -> bun_sys::Result { + reject_empty_path(path.as_bytes(), bun_sys::Tag::open)?; #[cfg(windows)] { use bun_sys::FdExt; diff --git a/test/js/bun/shell/commands/cp.test.ts b/test/js/bun/shell/commands/cp.test.ts index 80a28d2c7e2c..df217fa543b1 100644 --- a/test/js/bun/shell/commands/cp.test.ts +++ b/test/js/bun/shell/commands/cp.test.ts @@ -1,7 +1,9 @@ import { $ } from "bun"; import { shellInternals } from "bun:internal-for-testing"; -import { describe, expect } from "bun:test"; -import { tempDirWithFiles } from "harness"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, tempDir, tempDirWithFiles } from "harness"; +import { readFileSync, readdirSync } from "node:fs"; +import { join } from "node:path"; import { bunExe, createTestBuilder } from "../test_builder"; import { sortedShellOutput } from "../util"; const { builtinDisabled } = shellInternals; @@ -173,6 +175,55 @@ describe.if(!builtinDisabled("cp"))("bunshell cp", async () => { }); }); +// The builtin is the default only on Windows; on POSIX it is enabled by an env +// var that is read once per process, so each of these runs cp in a child bun. +describe.concurrent("bunshell cp with an empty operand", () => { + const builtinEnv = { ...bunEnv, BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS: "1" }; + + /** Runs `command` through the shell in `cwd`; the child prints cp's exit code, then its stderr. */ + async function cp(cwd: string, command: string) { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const result = await Bun.$\`\${{ raw: ${JSON.stringify(command)} }}\`.nothrow().quiet(); + process.stdout.write(result.exitCode + "\\n" + result.stderr.toString() + result.stdout.toString());`, + ], + cwd, + env: builtinEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + const ENOENT = "cp: No such file or directory\n"; + + // An empty operand used to be joined onto the shell's cwd, which resolved to + // the cwd itself: `cp "" out` complained that "" is a directory, `cp -R "" out` + // copied the cwd into itself, and `cp f ""` copied f onto itself and exited 0. + test.each([ + ['cp "" out', ENOENT], + ['cp -R "" out', ENOENT], + ['cp f ""', ENOENT], + ['cp -R f ""', ENOENT], + ['cp f g ""', ENOENT + ENOENT], + ])("%s fails with ENOENT and copies nothing", async (command, stderr) => { + using dir = tempDir("shell-cp-empty-operand", { f: "F\n", g: "G\n" }); + + expect(await cp(String(dir), command)).toEqual({ stdout: `1\n${stderr}`, stderr: "", exitCode: 0 }); + expect(readdirSync(String(dir)).sort()).toEqual(["f", "g"]); + expect(readFileSync(join(String(dir), "f"), "utf8")).toBe("F\n"); + }); + + test("the other sources are still copied when one of them is empty", async () => { + using dir = tempDir("shell-cp-empty-operand-one-of-many", { f: "F\n", out: {} }); + + expect(await cp(String(dir), 'cp "" f out')).toEqual({ stdout: `1\n${ENOENT}`, stderr: "", exitCode: 0 }); + expect(readFileSync(join(String(dir), "out", "f"), "utf8")).toBe("F\n"); + }); +}); + function expectSortedOutput(expected: string) { return (stdout: string, tempdir: string) => expect(sortedShellOutput(stdout).join("\n")).toEqual( diff --git a/test/js/bun/shell/commands/ls.test.ts b/test/js/bun/shell/commands/ls.test.ts index 7f1fcd1ea96b..0d7a7c4f4de9 100644 --- a/test/js/bun/shell/commands/ls.test.ts +++ b/test/js/bun/shell/commands/ls.test.ts @@ -287,6 +287,26 @@ describe.concurrent("bunshell ls", () => { .run(); }); + // On Windows the shell's openat emulation used to resolve an empty operand + // to the cwd itself, so `ls ""` listed the cwd and exited 0. + test.each(['ls ""', 'ls -R ""'])("%s fails with ENOENT", async cmd => { + await TestBuilder.command`${{ raw: cmd }}` + .file("a", "") + .exitCode(1) + .stdout("") + .stderr("ls: No such file or directory\n") + .run(); + }); + + test("the other operands are still listed when one is empty", async () => { + await TestBuilder.command`ls a ""` + .file("a", "") + .exitCode(1) + .stdout("a\n") + .stderr("ls: No such file or directory\n") + .run(); + }); + test("invalid flag", async () => { await TestBuilder.command`ls -z` .exitCode(1) diff --git a/test/js/bun/shell/commands/mkdir.test.ts b/test/js/bun/shell/commands/mkdir.test.ts index 8fd48977b24e..b4b8f929cf6f 100644 --- a/test/js/bun/shell/commands/mkdir.test.ts +++ b/test/js/bun/shell/commands/mkdir.test.ts @@ -1,5 +1,7 @@ -import { expect, test } from "bun:test"; +import { $ } from "bun"; +import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +import { readdirSync } from "node:fs"; import { join } from "node:path"; // A relative operand was joined onto the cwd in a fixed 4096-byte buffer, so an @@ -75,3 +77,43 @@ test("operands longer than the path buffers are reported, not a crash", async () }); expect(exitCode).toBe(0); }); + +$.nothrow(); + +const ENOENT = "mkdir: No such file or directory\n"; + +describe.concurrent("bunshell mkdir", () => { + // An empty operand used to be joined onto the shell's cwd, which resolved to + // the cwd itself: `mkdir ""` reported the cwd as existing and `mkdir -p ""` + // exited 0. + test.each([ + ['mkdir ""', () => $`mkdir ""`], + ['mkdir ${""}', () => $`mkdir ${""}`], + ['mkdir -p ""', () => $`mkdir -p ""`], + ['mkdir -pv ""', () => $`mkdir -pv ""`], + ['mkdir -v ""', () => $`mkdir -v ""`], + ])("%s fails with ENOENT", async (_name, command) => { + using dir = tempDir("mkdir-empty", {}); + const cwd = String(dir); + + const { stdout, stderr, exitCode } = await command().cwd(cwd).quiet(); + + expect(stdout.toString()).toBe(""); + expect(stderr.toString()).toBe(ENOENT); + expect(exitCode).toBe(1); + expect(readdirSync(cwd)).toEqual([]); + }); + + test("the other operands are still created when one is empty", async () => { + using dir = tempDir("mkdir-empty-multi", {}); + const cwd = String(dir); + + const { stdout, stderr, exitCode } = await $`mkdir -p a "" b/c`.cwd(cwd).quiet(); + + expect(stdout.toString()).toBe(""); + expect(stderr.toString()).toBe(ENOENT); + expect(exitCode).toBe(1); + expect(readdirSync(cwd).sort()).toEqual(["a", "b"]); + expect(readdirSync(join(cwd, "b"))).toEqual(["c"]); + }); +}); diff --git a/test/js/bun/shell/commands/mv.test.ts b/test/js/bun/shell/commands/mv.test.ts index 4389044ae861..0def753612c9 100644 --- a/test/js/bun/shell/commands/mv.test.ts +++ b/test/js/bun/shell/commands/mv.test.ts @@ -67,6 +67,46 @@ describe("mv", async () => { .stderr("mv: a: Not a directory\n") .runAsTest("move dir -> file fails"); + // On Windows the shell's fd-relative open/rename emulation used to resolve an + // empty operand to the cwd itself: `mv a ""` moved a into the cwd (a no-op + // that exited 0) and `mv "" b` tried to rename the cwd. + describe("empty operand", () => { + TestBuilder.command`mv a ""` + .ensureTempDir() + .file("a", "A\n") + .exitCode(2 /* ENOENT */) + .stderr("mv: No such file or directory\n") + .fileEquals("a", "A\n") + .runAsTest("as the destination fails"); + + TestBuilder.command`mv "" b` + .ensureTempDir() + .file("a", "A\n") + .exitCode(2 /* ENOENT */) + .stderr("mv: No such file or directory\n") + .fileEquals("a", "A\n") + .doesNotExist("b") + .runAsTest("as the source fails"); + + TestBuilder.command`mkdir d; mv "" d` + .ensureTempDir() + .file("a", "A\n") + .exitCode(2 /* ENOENT */) + .stderr("mv: d: No such file or directory\n") + .fileEquals("a", "A\n") + .runAsTest("as a source moved into a directory fails"); + + TestBuilder.command`mv a b ""` + .ensureTempDir() + .file("a", "A\n") + .file("b", "B\n") + .exitCode(1) + .stderr("mv: : No such file or directory\n") + .fileEquals("a", "A\n") + .fileEquals("b", "B\n") + .runAsTest("as the directory for several sources fails"); + }); + // POSIX `mv` must fall back to copy+unlink when `rename()` returns EXDEV // (source and destination on different filesystems). Requires a writable // mount on a different device from the harness temp dir. diff --git a/test/js/bun/shell/commands/rm.test.ts b/test/js/bun/shell/commands/rm.test.ts index 414c3b6ad5db..2e8d9a6dee07 100644 --- a/test/js/bun/shell/commands/rm.test.ts +++ b/test/js/bun/shell/commands/rm.test.ts @@ -6,8 +6,8 @@ */ import { $ } from "bun"; import { beforeAll, describe, expect, setDefaultTimeout, test } from "bun:test"; -import { bunEnv, bunExe, isWindows, tempDir } from "harness"; -import { existsSync, mkdirSync, renameSync, symlinkSync, writeFileSync } from "node:fs"; +import { bunEnv, bunExe, isPosix, isWindows, tempDir } from "harness"; +import { existsSync, mkdirSync, readdirSync, renameSync, symlinkSync, writeFileSync } from "node:fs"; import path from "path"; import { createTestBuilder, sortedShellOutput } from "../util"; const TestBuilder = createTestBuilder(import.meta.path); @@ -484,3 +484,59 @@ test("operands longer than the path scratch buffers are reported, not a crash", }); expect(exitCode).toBe(0); }); + +// An empty operand used to stand for the cwd twice over: the refuse-to-remove- +// the-root check joined it onto the cwd (so in a top-level directory `rm ""` +// refused to remove the cwd), and on Windows the fd-relative unlink/open +// emulation resolved it to the cwd itself, so `rm -r ""` emptied the cwd. +describe.concurrent("rm with an empty operand", () => { + const files = { f: "F\n", "sub/inner": "I\n" }; + const ENOENT = "rm: No such file or directory\n"; + + test.each([ + ['rm ""', { stdout: "", stderr: ENOENT, exitCode: 1 }], + ['rm -r ""', { stdout: "", stderr: ENOENT, exitCode: 1 }], + ['rm -d ""', { stdout: "", stderr: ENOENT, exitCode: 1 }], + ['rm -f ""', { stdout: "", stderr: "", exitCode: 0 }], + ['rm -rf ""', { stdout: "", stderr: "", exitCode: 0 }], + ])("%s removes nothing", async (command, expected) => { + using dir = tempDir("rm-empty-operand", files); + + const { stdout, stderr, exitCode } = await $`${{ raw: command }}`.cwd(String(dir)).quiet(); + + expect({ stdout: stdout.toString(), stderr: stderr.toString(), exitCode }).toEqual(expected); + expect(readdirSync(String(dir)).sort()).toEqual(["f", "sub"]); + expect(existsSync(path.join(String(dir), "sub", "inner"))).toBeTrue(); + }); + + test("the other operands are still removed when one is empty", async () => { + using dir = tempDir("rm-empty-operand-multi", files); + + const { stdout, stderr, exitCode } = await $`rm "" f`.cwd(String(dir)).quiet(); + + expect({ stdout: stdout.toString(), stderr: stderr.toString(), exitCode }).toEqual({ + stdout: "", + stderr: ENOENT, + exitCode: 1, + }); + expect(readdirSync(String(dir))).toEqual(["sub"]); + }); + + // /tmp is a top-level directory, which is where the root check used to trip + // on "". Nothing can be removed here: "" names nothing and is the only operand. + test.if(isPosix)("is not mistaken for the cwd by the root check in a top-level directory", async () => { + const forced = await $`rm -f ""`.cwd("/tmp").quiet(); + expect({ stdout: forced.stdout.toString(), stderr: forced.stderr.toString(), exitCode: forced.exitCode }).toEqual({ + stdout: "", + stderr: "", + exitCode: 0, + }); + + const plain = await $`rm ""`.cwd("/tmp").quiet(); + expect({ stdout: plain.stdout.toString(), stderr: plain.stderr.toString(), exitCode: plain.exitCode }).toEqual({ + stdout: "", + stderr: ENOENT, + exitCode: 1, + }); + }); +}); diff --git a/test/js/bun/shell/commands/touch.test.ts b/test/js/bun/shell/commands/touch.test.ts index 8c5e69391006..c0cc51f8f501 100644 --- a/test/js/bun/shell/commands/touch.test.ts +++ b/test/js/bun/shell/commands/touch.test.ts @@ -1,5 +1,7 @@ -import { expect, test } from "bun:test"; +import { $ } from "bun"; +import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +import { existsSync, statSync, utimesSync } from "node:fs"; import { join } from "node:path"; // Every operand, absolute or not, was joined into a fixed-size path buffer, so @@ -61,3 +63,52 @@ test("operands longer than the path buffer are reported, not a crash", async () }); expect(exitCode).toBe(0); }); + +$.nothrow(); + +const ENOENT = "touch: No such file or directory\n"; +const past = new Date("2000-01-01T00:00:00Z"); + +describe.concurrent("bunshell touch", () => { + // An empty operand used to be joined onto the shell's cwd, which resolved to + // the cwd itself: `touch ""` exited 0 and bumped the cwd's timestamps. + test('touch "" fails and leaves the cwd untouched', async () => { + using dir = tempDir("touch-empty", {}); + const cwd = String(dir); + utimesSync(cwd, past, past); + + const { stdout, stderr, exitCode } = await $`touch ""`.cwd(cwd).quiet(); + + expect(stdout.toString()).toBe(""); + expect(stderr.toString()).toBe(ENOENT); + expect(exitCode).toBe(1); + expect(statSync(cwd).mtimeMs).toBe(past.getTime()); + }); + + test('touch ${""} fails and leaves the cwd untouched', async () => { + using dir = tempDir("touch-empty-interp", {}); + const cwd = String(dir); + utimesSync(cwd, past, past); + + const { stdout, stderr, exitCode } = await $`touch ${""}`.cwd(cwd).quiet(); + + expect(stdout.toString()).toBe(""); + expect(stderr.toString()).toBe(ENOENT); + expect(exitCode).toBe(1); + expect(statSync(cwd).mtimeMs).toBe(past.getTime()); + }); + + test("the other operands are still touched when one is empty", async () => { + using dir = tempDir("touch-empty-multi", { existing: "" }); + const cwd = String(dir); + utimesSync(join(cwd, "existing"), past, past); + + const { stdout, stderr, exitCode } = await $`touch created "" existing`.cwd(cwd).quiet(); + + expect(stdout.toString()).toBe(""); + expect(stderr.toString()).toBe(ENOENT); + expect(exitCode).toBe(1); + expect(existsSync(join(cwd, "created"))).toBeTrue(); + expect(statSync(join(cwd, "existing")).mtimeMs).toBeGreaterThan(past.getTime()); + }); +});