shell(rm): stop the recursive walk from aborting on entries deeper than PATH_MAX

src/runtime/shell/builtin/rm.rs

@@ -853,12 +853,18 @@ impl ShellRmTask {
         Ok(())
     }
 
-    /// Join into `buf` honoring [`join_style`].
-    fn buf_join<'a>(&self, buf: &'a mut bun_paths::PathBuffer, parts: &[&[u8]]) -> &'a ZStr {
+    /// Join into `buf` (into `spill` when the result does not fit) honoring
+    /// [`join_style`].
+    fn buf_join<'a>(
+        &self,
+        buf: &'a mut bun_paths::PathBuffer,
+        spill: &'a mut Vec<u8>,
+        parts: &[&[u8]],
+    ) -> &'a ZStr {
         if self.join_style == JoinStyle::Posix {
-            resolve_path::join_z_buf::<platform::Posix>(buf.as_mut_slice(), parts)
+            resolve_path::join_z_buf_spill::<platform::Posix>(buf.as_mut_slice(), spill, parts)
         } else {
-            resolve_path::join_z_buf::<platform::Windows>(buf.as_mut_slice(), parts)
+            resolve_path::join_z_buf_spill::<platform::Windows>(buf.as_mut_slice(), spill, parts)
         }
     }
 
@@ -892,7 +898,9 @@ impl ShellRmTask {
             }
             return ZBox::from_vec(out);
         }
-        ZBox::from_bytes(resolve_path::join::<platform::Auto>(parts))
+        let mut spill = Vec::new();
+        let joined = resolve_path::join_spill::<platform::Auto>(&mut spill, parts);
+        ZBox::from_bytes(joined)
     }
 
     #[inline]
@@ -1032,6 +1040,7 @@ impl ShellRmTask {
         // `delete_after_waiting_for_children` on this dir and the owning
         // `ShellRmTask` would never be freed.
         let mut i: usize = 0;
+        let mut join_spill = Vec::new();
         let loop_result: bun_sys::Maybe<()> = loop {
             let current = match iterator.next() {
                 Err(e) => break Err(self.error_with_path(&e, path.as_bytes())),
@@ -1057,7 +1066,7 @@ impl ShellRmTask {
                     // Copy the join into an owned ZBox so `buf` is free to
                     // be re-borrowed by the vtable callback.
                     let file_path = {
-                        let joined = self.buf_join(buf, &[path.as_bytes(), name]);
+                        let joined = self.buf_join(buf, &mut join_spill, &[path.as_bytes(), name]);
                         ZBox::from_bytes(joined.as_bytes())
                     };
                     if let Err(e) = self.remove_entry_file(

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

@@ -290,6 +290,99 @@ test.skipIf(process.platform === "win32")(
   },
 );
 
+// The recursive walk joined every entry onto its directory's path inside a
+// fixed-size path buffer on the worker thread, so a tree deeper than PATH_MAX
+// aborted the whole process instead of failing that entry. Files and
+// directories take different joins, so one tree of each. Runs in a child
+// process so the abort shows up as a failed assertion. Windows has a
+// different path limit, and its shell rm is bounded differently.
+test.skipIf(process.platform === "win32")(
+  "recursive rm reports an entry deeper than PATH_MAX instead of crashing",
+  async () => {
+    // Linux PATH_MAX is 4096, every other POSIX platform Bun runs on has 1024.
+    const PATH_MAX = process.platform === "linux" ? 4096 : 1024;
+    using base = tempDir("rm-deep-walk", {});
+    const fixture = /* ts */ `
+      import { $ } from "bun";
+      import { existsSync, mkdirSync, writeFileSync } from "node:fs";
+      import { join } from "node:path";
+      $.nothrow();
+
+      const base = process.env.BASE!;
+      // Long enough that a 100 byte entry inside the directory is past
+      // PATH_MAX, short enough that the directory itself is still well inside.
+      const deepLength = Number(process.env.PATH_MAX) - 64;
+      const component = Buffer.alloc(100, "d").toString();
+      // <base>/<tag>/ddd.../ddd... with an absolute path of deepLength bytes.
+      function deepDir(tag: string): string {
+        let dir = join(base, tag);
+        while (dir.length + 1 + component.length <= deepLength) dir = join(dir, component);
+        const rest = deepLength - dir.length - 1;
+        if (rest > 0) dir = join(dir, Buffer.alloc(rest, "e").toString());
+        mkdirSync(dir, { recursive: true });
+        return dir;
+      }
+
+      // An entry whose full path is longer than PATH_MAX can only be created
+      // relative to its directory.
+      const fileDir = deepDir("file");
+      const fileName = Buffer.alloc(100, "f").toString();
+      process.chdir(fileDir);
+      writeFileSync(fileName, "");
+
+      const dirDir = deepDir("dir");
+      const dirName = Buffer.alloc(100, "s").toString();
+      process.chdir(dirDir);
+      mkdirSync(dirName);
+
+      process.chdir(base);
+      mkdirSync(join(base, "plain", "sub"), { recursive: true });
+
+      const run = async (operand: string) => {
+        const { exitCode, stderr } = await $\`rm -rf \${operand}\`.cwd(base).quiet();
+        return { exitCode, stderr: stderr.toString() };
+      };
+      console.log(
+        JSON.stringify({
+          file: { ...(await run(join(base, "file"))), entry: join(fileDir, fileName), dirKept: existsSync(fileDir) },
+          dir: { ...(await run(join(base, "dir"))), entry: join(dirDir, dirName), dirKept: existsSync(dirDir) },
+          plain: { ...(await run(join(base, "plain"))), removed: !existsSync(join(base, "plain")) },
+        }),
+      );
+    `;
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", fixture],
+      env: { ...bunEnv, BASE: String(base), PATH_MAX: String(PATH_MAX) },
+      cwd: String(base),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(stderr).toBe("");
+    const results = JSON.parse(stdout);
+    // The entries are what rm could not remove, and this is why: a path of
+    // PATH_MAX bytes has no room left for its NUL.
+    expect(results.file.entry.length).toBeGreaterThanOrEqual(PATH_MAX);
+    expect(results.dir.entry.length).toBeGreaterThanOrEqual(PATH_MAX);
+    expect(results).toEqual({
+      file: {
+        exitCode: 1,
+        stderr: `rm: ${results.file.entry}: File name too long\n`,
+        entry: expect.stringMatching(/\/f{100}$/),
+        dirKept: true,
+      },
+      dir: {
+        exitCode: 1,
+        stderr: `rm: ${results.dir.entry}: File name too long\n`,
+        entry: expect.stringMatching(/\/s{100}$/),
+        dirKept: true,
+      },
+      plain: { exitCode: 0, stderr: "", removed: true },
+    });
+    expect(exitCode).toBe(0);
+  },
+);
+
 test.skipIf(process.platform === "win32")(
   "relative operands are resolved against the shell cwd, not the process cwd",
   async () => {