From 91a0fa0d62c73678cb0038cba9b3067b2434c862 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 03:04:11 +0000 Subject: [PATCH] shell: report the operand as written in cat, touch and mkdir errors The builtin cat (on Windows), touch and mkdir (everywhere) resolve the operand against the cwd before the syscall and tagged the error with that resolved path, so "mkdir nodir/a" printed "mkdir: /tmp/x/nodir/a: No such file or directory" while ls, rm, mv and the coreutils name the operand as the user wrote it. shell_openat's Windows file branch now re-tags the open error with the caller's path like its sibling branches; touch and mkdir tag the operand they were given instead of the joined path (mkdir strips the NUL its absolute branch appends in place). The contract is noted on Builtin::task_error_to_string, the one printer of err.path. The new test runs cat, touch and mkdir (including mkdir -p and a touch whose utimes() fails outright) against relative, nested, rooted and absolute operands plus a 2> redirect each, and the cat completion test now pins the exact message it previously had to accept either form of. --- src/runtime/shell/Builtin.rs | 1 + src/runtime/shell/builtin/mkdir.rs | 9 +++- src/runtime/shell/builtin/touch.rs | 4 +- src/runtime/shell/interpreter.rs | 2 +- test/js/bun/shell/bunshell.test.ts | 87 ++++++++++++++++++++++++++++-- 5 files changed, 95 insertions(+), 8 deletions(-) diff --git a/src/runtime/shell/Builtin.rs b/src/runtime/shell/Builtin.rs index 6ce494deb4ed..ce32ddc18c03 100644 --- a/src/runtime/shell/Builtin.rs +++ b/src/runtime/shell/Builtin.rs @@ -1033,6 +1033,7 @@ impl Builtin { /// `bun_sys::coreutils_error_map` so output matches GNU coreutils /// (e.g. `ENOENT` → "No such file or directory"); falls back to /// `"unknown error {errno}"` when unmapped. + /// `err.path` is printed as the operand, so builtins that resolve an operand tag it as written. pub(crate) fn task_error_to_string<'a>( interp: &'a Interpreter, cmd: NodeId, diff --git a/src/runtime/shell/builtin/mkdir.rs b/src/runtime/shell/builtin/mkdir.rs index a4695d61c6b2..a2de45e792fb 100644 --- a/src/runtime/shell/builtin/mkdir.rs +++ b/src/runtime/shell/builtin/mkdir.rs @@ -294,6 +294,11 @@ impl ShellMkdirTask { bun_core::heap::into_raw(task) } + /// The operand as written; `run_from_thread_pool` NUL-terminates an absolute one in place. + fn operand(&self) -> &[u8] { + self.filepath.strip_suffix(b"\0").unwrap_or(&self.filepath) + } + fn run_from_thread_pool(this: &mut ShellMkdirTask) { use bun_paths::{Platform, platform, resolve_path}; // We have to give an absolute path to our mkdir implementation for it @@ -325,7 +330,7 @@ impl ShellMkdirTask { active: this.opts.verbose, }; if let Err(e) = node_fs.mkdir_recursive_impl(&args, &vtable) { - this.err = Some(e.with_path(filepath.as_bytes())); + this.err = Some(e.with_path(this.operand())); core::hint::black_box(&node_fs); } } else { @@ -338,7 +343,7 @@ impl ShellMkdirTask { } } Err(e) => { - this.err = Some(e.with_path(filepath.as_bytes())); + this.err = Some(e.with_path(this.operand())); core::hint::black_box(&node_fs); } } diff --git a/src/runtime/shell/builtin/touch.rs b/src/runtime/shell/builtin/touch.rs index 95ae1398deee..1408ebe4de49 100644 --- a/src/runtime/shell/builtin/touch.rs +++ b/src/runtime/shell/builtin/touch.rs @@ -296,12 +296,12 @@ impl ShellTouchTask { break 'out; } Err(e) => { - this.err = Some(e.with_path(filepath.as_bytes())); + this.err = Some(e.with_path(&this.filepath)); break 'out; } } } - this.err = Some(err.with_path(filepath.as_bytes())); + this.err = Some(err.with_path(&this.filepath)); } } // Worker→main bounce-back is posted by `shell_task_trampoline` after diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index f0e6da780e48..24aacb0422c9 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -2290,7 +2290,7 @@ pub(crate) fn shell_openat( let p = shell_get_path(dir, path, &mut buf)?; // No `makeLibUVOwnedForSyscall` here: `bun_sys::open` on Windows // routes through `sys_uv` and already yields a uv-owned fd. - return bun_sys::open(p, flags, perm); + return bun_sys::open(p, flags, perm).map_err(|e| e.with_path(path.as_bytes())); } #[cfg(not(windows))] { diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 2d57ca0587c9..7d7c28a7b435 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -91,6 +91,89 @@ describe("bunshell", () => { ); }); + // touch and mkdir (everywhere) and cat (on Windows) resolve the operand + // against the cwd before the syscall; the error must still name the operand + // as the user wrote it, like ls/rm/mv and the system coreutils do. The flag + // turns the builtin cat on outside Windows; touch and mkdir are always + // builtins. + test("builtins name a failing operand as written", async () => { + using dir = tempDir("builtin-operand-as-written", { sub: {}, afile: "" }); + const enoent = "No such file or directory"; + const enotdir = "Not a directory"; + // The operand is the last word of each command; stderr must be exactly + // `: : `. + const rows: [command: string[], message: string][] = [ + [["cat", "missing.txt"], enoent], + [["cat", "sub/missing.txt"], enoent], + [["cat", "/bunshell-missing-operand/missing.txt"], enoent], + [["cat", join(String(dir), "missing.txt")], enoent], + [["touch", "nodir/file.txt"], enoent], + [["touch", "sub/nodir/file.txt"], enoent], + [["touch", "/bunshell-missing-operand/file.txt"], enoent], + [["touch", join(String(dir), "nodir", "file.txt")], enoent], + // A missing parent fails in the open() touch falls back to; a file in + // the way already fails in utimes() (Windows reports that as ENOENT too). + [["touch", "afile/file.txt"], isWindows ? enoent : enotdir], + [["mkdir", "nodir/child"], enoent], + [["mkdir", "sub/nodir/child"], enoent], + [["mkdir", "/bunshell-missing-operand/child"], enoent], + [["mkdir", join(String(dir), "nodir", "child")], enoent], + // -p is a separate code path; like before, it names the operand rather + // than the component that failed. + [["mkdir", "-p", "afile/child"], enotdir], + ]; + // The first row of each builtin is also run with stderr redirected to a + // file, which takes the builtins' other output path. + const redirected = ["cat", "touch", "mkdir"].map(builtin => rows.find(([command]) => command[0] === builtin)!); + + const script = /* ts */ ` + import { $ } from "bun"; + $.nothrow(); + const results = {}; + const record = async (key, promise) => { + const r = await promise.quiet(); + results[key] = { stdout: r.stdout.toString(), stderr: r.stderr.toString(), exitCode: r.exitCode }; + }; + for (const command of ${JSON.stringify(rows.map(([command]) => command))}) { + const words = command.slice(0, -1).join(" "); + const operand = command[command.length - 1]; + await record(command.join(" "), $\`\${{ raw: words }} \${operand}\`); + } + for (const command of ${JSON.stringify(redirected.map(([command]) => command))}) { + const words = command.slice(0, -1).join(" "); + const operand = command[command.length - 1]; + const errFile = "err-" + command[0] + ".txt"; + await record(command.join(" ") + " 2> " + errFile, $\`\${{ raw: words }} \${operand} 2> \${{ raw: errFile }}\`); + } + console.log(JSON.stringify(results)); + `; + await using proc = Bun.spawn({ + cmd: [BUN, "-e", script], + env: { ...bunEnv, BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS: "1" }, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + + const errorLine = (command: string[], message: string) => + `${command[0]}: ${command[command.length - 1]}: ${message}\n`; + const results = JSON.parse(stdout); + const expected: Record = {}; + for (const [command, message] of rows) { + expected[command.join(" ")] = { stdout: "", stderr: errorLine(command, message), exitCode: 1 }; + } + for (const [command, message] of redirected) { + const errFile = `err-${command[0]}.txt`; + expected[`${command.join(" ")} 2> ${errFile}`] = { stdout: "", stderr: "", exitCode: 1 }; + expected[errFile] = errorLine(command, message); + results[errFile] = await Bun.file(join(String(dir), errFile)).text(); + } + expect(results).toEqual(expected); + expect(exitCode).toBe(0); + }); + describe("concurrency", () => { test("writing to stdout", async () => { await Promise.all([ @@ -560,9 +643,7 @@ describe("bunshell", () => { }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - // On Windows the message carries the absolute path (shell_openat only - // re-tags the error with the argument as written on POSIX). - const missingFileError = expect.stringMatching(/^cat: (.*[\\/])?missing\.txt: No such file or directory\n$/); + const missingFileError = "cat: missing.txt: No such file or directory\n"; expect(JSON.parse(stdout)).toEqual({ "captured": { stdout: "hi\n", stderr: "", exitCode: 0 }, "stdout to fd": { stdout: "", stderr: "", exitCode: 0 },