fs: fix repeated copy of directory with symlinks by haramj · Pull Request #65411 · nodejs/node

haramj · 2026-08-20T01:39:48Z

Repeatedly copying a directory that contains a symlink to an unrelated
directory fails on the second copy with ERR_FS_CP_EINVAL because the
symlink target is mistaken for a self-referential copy.

Root cause

Both the JS (isSrcSubdir in lib/internal/fs/cp/cp.js) and the C++
(isInsideDir in src/node_file.cc) guards are prefix-inclusive, so when
the source symlink and the already-copied destination symlink resolve to
the same directory, the "copy to a subdirectory of self" check fires on
the identical target.

Fix

Treat identical symlink targets as a self-copy only when the target is
the destination root or one of its ancestors; identical targets that are
unrelated to the destination root are allowed to be copied again.
Compare targets using canonicalized paths (weakly_canonical) in C++
so relative-vs-absolute representations and macOS /var vs
/private/var are handled correctly.
Thread the top-level destination root through the JS copy recursion via
the internal options object.

Unlike the earlier attempt in #65099, the existing self-referential
protections are kept: a symlink pointing to the destination root or an
ancestor of it still fails with ERR_FS_CP_EINVAL.

Tests

New regression tests (test-fs-cp-sync-symlink-repeated-copy and
test-fs-cp-async-symlink-repeated-copy) cover both the C++ (no filter)
and JS (filter) paths with absolute and relative targets, each copied
twice, and verify the copied symlink still resolves to the original
target. Existing *-points-to-dest tests are unchanged and keep passing.

Validation

make builds successfully.
All test/parallel/test-fs-*.{mjs,js} (351 tests) pass, including the
new regression tests and all *-points-to-dest tests.
ESLint clean; git-clang-format reports no formatting changes.

codecov · 2026-08-20T03:28:24Z

Codecov Report

❌ Patch coverage is 87.69231% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.35%. Comparing base (75e4bbe) to head (239d414).

Files with missing lines	Patch %	Lines
src/node_file.cc	82.92%	3 Missing and 4 partials ⚠️
lib/internal/fs/cp/cp-sync.js	92.30%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65411      +/-   ##
==========================================
- Coverage   90.36%   90.35%   -0.02%     
==========================================
  Files         792      792              
  Lines      275398   275441      +43     
  Branches    52776    52798      +22     
==========================================
+ Hits       248877   248887      +10     
- Misses      16937    16963      +26     
- Partials     9584     9591       +7

Files with missing lines	Coverage Δ
lib/internal/fs/cp/cp.js	`93.28% <100.00%> (+0.14%)`	⬆️
lib/internal/fs/cp/cp-sync.js	`81.95% <92.30%> (+10.35%)`	⬆️
src/node_file.cc	`75.25% <82.92%> (+0.06%)`	⬆️

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

haramj · 2026-08-20T06:59:15Z

Hi @RafaelGSS @jasnell @Renegade334,

I've updated the PR to also cover the JavaScript (filter) path in the repeated-copy regression tests — cp-sync.js coverage for the new symlink-target logic is now complete. All CI checks pass.

Would you mind taking a look when you have a moment? Thanks!

RafaelGSS · 2026-09-25T20:34:58Z

+              bool same_target =
+                  symlink_target_absolute ==
+                      current_dest_symlink_target_absolute &&
+                  !isInsideDir(symlink_target_absolute, dest_path);


symlink_target_absolute has gone through weakly_canonical, but dest_path is the non-canonical path passed from JS.

If dest is reached through a symlinked directory (e.g. os.tmpdir() on macOS, /var -> /private/var), isInsideDir() returns false for a link pointing at dest itself, so same_target becomes true and the copy succeeds here

RafaelGSS · 2026-09-25T20:34:58Z

+              if (error) {
+                return CpError::Std(error, dest_str);
+              }
+#ifdef _WIN32


(Non-blocking) This duplicates the \\?\ stripping just above and in normalizePathToArray(). A small helper would keep the three in sync.

RafaelGSS · 2026-09-25T20:34:58Z

+mkdirSync(src, mustNotMutateObjectDeep({ recursive: true }));
+mkdirSync(target);
+symlinkSync(target, join(src, 'link'));
+cp(src, dest, mustNotMutateObjectDeep({ recursive: true }), mustCall((err) => {


(Non-blocking) mustSucceed() instead of mustCall((err) => { assert.ifError(err); ... }). Since this is an .mjs file, using fs/promises cp with top-level await would also avoid the four levels of nesting.

RafaelGSS · 2026-09-25T20:34:58Z

+  // A symlink that resolves to the same target as the destination symlink
+  // is not a self-copy, unless the target is the destination directory
+  // itself or one of its ancestors.
+  const sameTarget = resolvedSrc === resolvedDest &&


This also changes behavior for an identical target that lives inside dest (e.g. dest/sub): it used to throw ERR_FS_CP_EINVAL and is now allowed. That seems correct, since replacing a link with an identical one is a no-op, but could you add a test for it and for a target that is an ancestor of dest (not only dest itself)?

Repeatedly copying a directory that contains a symlink to an unrelated directory fails on the second copy with ERR_FS_CP_EINVAL because the symlink target is mistaken for a self-referential copy. Compare symlink targets using canonicalized paths, treating identical targets as self-copies only when the target is the destination root or one of its ancestors. Fixes: nodejs#65097 Signed-off-by: haramjeong <04harams77@gmail.com> Assisted-by: Codex

Canonicalize the destination root before allowing identical symlink replacements, and use real paths for the corresponding JavaScript guard. Share Windows namespace normalization and simplify the async tests. Cover descendant, equal, and ancestor targets through direct and aliased destinations with absolute and relative links, with and without filters. Signed-off-by: haramjeong <04harams77@gmail.com> Assisted-by: Codex

haramj · 2026-09-28T04:35:11Z

Thanks for the review, @RafaelGSS! I've addressed your feedback and added the requested tests. All 30 CI checks are now passing. Could you please take another look when you have a chance?

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run. labels Aug 20, 2026

haramj force-pushed the fix/65097-cp-repeated-symlink-copy branch from f3190d6 to b91d82d Compare August 20, 2026 01:48

haramj force-pushed the fix/65097-cp-repeated-symlink-copy branch from b91d82d to 7820495 Compare August 20, 2026 05:10

haramj force-pushed the fix/65097-cp-repeated-symlink-copy branch from 7820495 to 9ff389d Compare August 27, 2026 07:24

haramj requested review from RafaelGSS, addaleax and trivikr August 28, 2026 00:19

haramj force-pushed the fix/65097-cp-repeated-symlink-copy branch from 9ff389d to 375eaa8 Compare September 7, 2026 07:15

haramj force-pushed the fix/65097-cp-repeated-symlink-copy branch from 375eaa8 to 25cbae4 Compare September 25, 2026 14:05

RafaelGSS requested changes Sep 25, 2026

View reviewed changes

haramj force-pushed the fix/65097-cp-repeated-symlink-copy branch from 25cbae4 to d5ef584 Compare September 28, 2026 00:46

haramj force-pushed the fix/65097-cp-repeated-symlink-copy branch from d5ef584 to 239d414 Compare September 28, 2026 00:47

haramj requested a review from RafaelGSS September 28, 2026 04:34

Navigation Menu

Uh oh!

fs: fix repeated copy of directory with symlinks - #65411

fs: fix repeated copy of directory with symlinks#65411
haramj wants to merge 2 commits into
nodejs:mainfrom
haramj:fix/65097-cp-repeated-symlink-copy

haramj commented Aug 20, 2026

Uh oh!

codecov Bot commented Aug 20, 2026 •

edited

Loading

Uh oh!

haramj commented Aug 20, 2026

Uh oh!

RafaelGSS Sep 25, 2026 •

edited

Loading

Uh oh!

RafaelGSS Sep 25, 2026

Uh oh!

RafaelGSS Sep 25, 2026

Uh oh!

RafaelGSS Sep 25, 2026

Uh oh!

haramj commented Sep 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

haramj commented Aug 20, 2026

Root cause

Fix

Tests

Validation

Uh oh!

codecov Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

haramj commented Aug 20, 2026

Uh oh!

RafaelGSS Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RafaelGSS Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

RafaelGSS Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

RafaelGSS Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

haramj commented Sep 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Aug 20, 2026 •

edited

Loading

RafaelGSS Sep 25, 2026 •

edited

Loading