test: move the recursive cp error race test to the shell cp builtin

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, DirectoryTree, isWindows, tempDir, tempDirWithFiles } from "harness";
+import { 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,67 @@ describe.if(!builtinDisabled("cp"))("bunshell cp", async () => {
   });
 });
 
+// The builtin's `cp -R` copies the files of a directory as concurrent work-pool
+// tasks sharing one parent task. A file copy that fails records its error on
+// the parent, which has to stay alive until the sibling copies still running
+// have let go of it (before oven-sh/bun#30162 it was freed as soon as the error
+// was recorded, a heap-use-after-free that ASAN builds abort on). fs.cp walks
+// directories in JS on Linux and Windows, so this builtin is what reaches that
+// code. The builtin is the default only on Windows; on POSIX it is enabled by
+// an env var, so the copies run in a child bun.
+test("cp -R survives a file copy failing while its siblings are still being copied", async () => {
+  const files: DirectoryTree = {
+    "src/000-bad.txt": "x",
+    // dst exists, so `cp -R src dst` copies into dst/src. A directory already
+    // sitting where 000-bad.txt goes makes that one file's copy fail.
+    "dst/src/000-bad.txt": {},
+  };
+  // Enough siblings that some are still in flight when 000-bad.txt fails.
+  for (let i = 0; i < 32; i++) files[`src/f${i}.txt`] = "x";
+  using dir = tempDir("shell-cp-failed-sibling", files);
+
+  await using proc = Bun.spawn({
+    cmd: [
+      bunExe(),
+      "-e",
+      `
+      const outcomes = new Set();
+      for (let i = 0; i < 20; i++) {
+        const { exitCode, stderr } = await Bun.$\`cp -R src dst\`.nothrow().quiet();
+        outcomes.add(JSON.stringify({ exitCode, stderr: stderr.toString() }));
+      }
+      console.log(JSON.stringify([...outcomes].map(outcome => JSON.parse(outcome))));
+      `,
+    ],
+    cwd: String(dir),
+    env: {
+      ...bunEnv,
+      BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS: "1",
+      // An ASAN abort (the pre-fix behaviour) spends several seconds in
+      // llvm-symbolizer, which would turn the failure into a timeout; the
+      // report header in stderr is what identifies it.
+      ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "symbolize=0"].filter(Boolean).join(":"),
+    },
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect(stderr).toBe("");
+  // Every run failed on that one file. The message is the builtin's; the
+  // system cp, which the shell falls back to without the env var, words it
+  // differently.
+  expect(JSON.parse(stdout)).toEqual([
+    {
+      exitCode: 1,
+      stderr: `cp: ${isWindows ? "Operation not permitted" : "Is a directory"}: ${join(String(dir), "dst", "src", "000-bad.txt")}\n`,
+    },
+  ]);
+  expect(exitCode).toBe(0);
+  // The failure does not stop the siblings from being copied, which is what
+  // leaves them touching the parent after the error.
+  expect(readdirSync(join(String(dir), "dst", "src"))).toHaveLength(33);
+});
+
 function expectSortedOutput(expected: string) {
   return (stdout: string, tempdir: string) =>
     expect(sortedShellOutput(stdout).join("\n")).toEqual(

test/js/node/fs/cp.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, jest, test } from "bun:test";
 import fs from "fs";
-import { bunEnv, bunExe, isLinux, isPosix, isWindows, tempDir } from "harness";
+import { bunEnv, bunExe, isLinux, isWindows, tempDir } from "harness";
 import { mkfifo } from "mkfifo";
 import { isAbsolute, join } from "path";
 
@@ -146,6 +146,20 @@ for (const [name, copy] of impls) {
       assertContent(basename + "/result/a.txt", "win");
     });
 
+    test("recursive directory structure - a file whose destination is a directory throws", async () => {
+      await using basename = tempDir("cp", {
+        "from/a.txt": "a",
+        "from/b.txt": "b",
+        "result/b.txt": {},
+      });
+
+      const e = await copyShouldThrow(basename + "/from", basename + "/result", { recursive: true });
+      expect(e.code).toBe("ERR_FS_CP_NON_DIR_TO_DIR");
+      // Entries are reported with the path cp built for them, not a caller string.
+      expect(e.path).toBe(join(String(basename), "result", "b.txt"));
+      expect(fs.statSync(basename + "/result/b.txt").isDirectory()).toBe(true);
+    });
+
     test("symlinks - single file", async () => {
       await using basename = tempDir("cp", {
         "from/a.txt": "a",
@@ -593,68 +607,6 @@ describe.skipIf(isWindows).each(["cp", "cpSync"] as const)(
   },
 );
 
-// fs.promises.cp recursive: when one SingleTask copy fails while siblings are
-// still in flight on the thread pool, the parent AsyncCpTask must not be
-// destroyed until every subtask has dropped its reference. Before the fix,
-// the failing subtask enqueued runFromJSThread immediately and the JS thread
-// freed the parent while other subtasks were still dereferencing it
-// (heap-use-after-free under ASAN).
-//
-// POSIX-only: uses a pre-existing directory at the destination path of one
-// file so that its SingleTask fails with EISDIR. This works even when running
-// as root. On macOS the pre-existing dst/ makes clonefile() fail with EEXIST
-// and fall through to the per-file SingleTask path being tested.
-test.skipIf(!isPosix)(
-  "fs.promises.cp recursive does not free parent task while subtasks are in flight after an error",
-  async () => {
-    const files: Record<string, string | object> = {};
-    // Enough siblings so several SingleTasks are running on the thread pool
-    // when the failing one errors.
-    for (let i = 0; i < 32; i++) files[`src/f${i}.txt`] = "x";
-    files["src/000-bad.txt"] = "x";
-    // The destination for 000-bad.txt is a directory → copying into it fails.
-    files["dst/000-bad.txt"] = { ".keep": "" };
-    using dir = tempDir("cp-uaf", files);
-    const base = String(dir);
-
-    // Run the copy in a subprocess: before the fix this is a
-    // heap-use-after-free that ASAN aborts on. The subprocess loops to make
-    // the race reliable. It must reject with EISDIR each iteration and exit 0.
-    const script = `
-      const fs = require("fs");
-      const path = require("path");
-      const base = ${JSON.stringify(base)};
-      const src = path.join(base, "src");
-      const dst = path.join(base, "dst");
-      (async () => {
-        for (let i = 0; i < 20; i++) {
-          try {
-            await fs.promises.cp(src, dst, { recursive: true });
-            console.log("UNEXPECTED-SUCCESS");
-            process.exit(1);
-          } catch (e) {
-            if (e?.code !== "ERR_FS_CP_NON_DIR_TO_DIR") {
-              console.log("UNEXPECTED-ERROR:" + (e?.code ?? e?.message));
-              process.exit(1);
-            }
-          }
-        }
-        console.log("ok");
-      })();
-    `;
-    await using proc = Bun.spawn({
-      cmd: [bunExe(), "-e", script],
-      env: bunEnv,
-      stdout: "pipe",
-      stderr: "pipe",
-    });
-    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
-    expect(stderr).toBe("");
-    expect(stdout.trim()).toBe("ok");
-    expect(exitCode).toBe(0);
-  },
-);
-
 test.skipIf(!isLinux)("fs.cp and fs.copyFile create the destination with the source file's mode", async () => {
   using dir = tempDir("cp-dest-mode", {});
   const destNames = ["dest-copyFile.bin", "dest-cp.bin"];