fs.cp and fs.cpSync fail to repeatedly copy directory with symlinks

[meatcar]

Version

v24.16.0

Platform

Linux machine 7.1.3-cachyos-lto #1-NixOS SMP PREEMPT_DYNAMIC Tue Jan  1 00:00:00 UTC 1980 x86_64 GNU/Linux

Subsystem

fs

What steps will reproduce the bug?

This script reproduces the issue:

root="$(mktemp -d)"
trap 'rm -rf "$root"' EXIT

mkdir "$root/dir" "$root/foo"
ln -s "$root/dir" "$root/foo/link"

ROOT="$root" node <<'NODE'
const fs = require("node:fs");
const root = process.env.ROOT;

fs.cpSync(`${root}/foo`, `${root}/bar`, { recursive: true }); // succeeds
fs.cpSync(`${root}/foo`, `${root}/bar`, { recursive: true }); // throws
NODE

How often does it reproduce? Is there a required condition?

Requires a copying repeatedly a dir containing a symlink to another dir.

What is the expected behavior? Why is that the expected behavior?

The second copy should succeed, replacing bar/link or leaving it unchanged.

What do you see instead?

node:internal/fs/cp/cp-sync:145
    return fsBinding.cpSyncCopyDir(src, dest,
                     ^

Error: Cannot copy /tmp/tmp.61ap1tNPej/dir to a subdirectory of self /tmp/tmp.61ap1tNPej/dir
    at copyDir (node:internal/fs/cp/cp-sync:145:22)
    at onDir (node:internal/fs/cp/cp-sync:137:10)
    at getStats (node:internal/fs/cp/cp-sync:68:12)
    at cpSyncFn (node:internal/fs/cp/cp-sync:58:10)
    at Object.cpSync (node:fs:3158:3)
    at [stdin]:5:4
    at runScriptInThisContext (node:internal/vm:219:10)
    at node:internal/process/execution:451:12
    at [stdin]-wrapper:6:24
    at runScriptInContext (node:internal/process/execution:449:60) {
  code: 'ERR_FS_CP_EINVAL'
}

Node.js v24.16.0

Additional information

Also recreated in:

• Node.js v22.23.1
• Node.js v26.7.0
• Node.js v27.0.0-nightly20260806de333e856d

The result is the same with force: true, force: false, or errorOnExist. fs.cp() and fs.promises.cp() are also affected.

Modifying bar/link, or pointing it to a different directory, makes the second copy succeed.

[skdas20]

Reproduced on v24.14.0. Posting the mechanism, because there's a conflict here that I think blocks the obvious fix — and I suspect it's why #65099 was withdrawn.

Mechanism. In onLink (lib/internal/fs/cp/cp.js), after both symlinks are resolved:

if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) {
  throw new ERR_FS_CP_EINVAL({ message: `cannot copy ${resolvedSrc} to a subdirectory of self ${resolvedDest}` });
}

On the second copy, foo/link and bar/link both resolve to the same target, so resolvedSrc === resolvedDest. isSrcSubdir compares path segments with every, so identical paths satisfy it — a path is treated as a subdirectory of itself. Hence the error message naming the same path twice:

Cannot copy /tmp/x/dir to a subdirectory of self /tmp/x/dir

Why "skip when the targets are equal" doesn't work. test/parallel/test-fs-cp-async-symlink-points-to-dest.mjs deliberately asserts the opposite for a case that reaches this line with equally resolved paths:

symlinkSync(dest, join(src, 'link'));
cpSync(src, dest, { recursive: true });      // creates dest/link -> dest
cp(src, dest, ..., (err) => assert.strictEqual(err.code, 'ERR_FS_CP_EINVAL'));

There resolvedSrc and resolvedDest are both dest. So equality alone cannot separate the two situations, and any fix keyed on it forces that existing test to be rewritten — which is what #65099 had to do, along with relaxing isSrcSubdir itself (used in three other places).

What actually differs. In this issue the symlink points at an unrelated directory (/tmp/x/dir) while the copy is foo → bar; re-copying is idempotent and should succeed. In the existing test the symlink points at the destination root itself, which is the self-referential case the guard is meant to catch. The distinguishing fact is the relationship between the symlink target and the destination root — not the relationship between the two resolved targets.

So a correct fix likely needs the destination root at this point, rather than a change to isSrcSubdir.

Before I write that: is the existing test's expectation still considered correct, or is it encoding the same defect from a different angle? I'd rather have that settled than silently redefine what ERR_FS_CP_EINVAL means here. Happy to implement whichever way it's meant to behave, with tests covering both shapes.