Skip to content

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

Open
haramj wants to merge 1 commit into
nodejs:mainfrom
haramj:fix/65097-cp-repeated-symlink-copy
Open

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

Conversation

@haramj

@haramj haramj commented Aug 20, 2026

Copy link
Copy Markdown
Member

Fixes: #65097

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.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Aug 20, 2026
@haramj
haramj force-pushed the fix/65097-cp-repeated-symlink-copy branch from f3190d6 to b91d82d Compare August 20, 2026 01:48
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.68293% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.13%. Comparing base (d6bbf57) to head (7820495).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/fs/cp/cp-sync.js 83.33% 2 Missing ⚠️
src/node_file.cc 94.73% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65411      +/-   ##
==========================================
- Coverage   90.13%   90.13%   -0.01%     
==========================================
  Files         752      752              
  Lines      251864   252307     +443     
  Branches    47354    47464     +110     
==========================================
+ Hits       227021   227411     +390     
- Misses      16180    16202      +22     
- Partials     8663     8694      +31     
Files with missing lines Coverage Δ
lib/internal/fs/cp/cp.js 90.07% <100.00%> (+1.72%) ⬆️
src/node_file.cc 74.01% <94.73%> (-0.26%) ⬇️
lib/internal/fs/cp/cp-sync.js 69.44% <83.33%> (+10.83%) ⬆️

... and 41 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.

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>
@haramj
haramj force-pushed the fix/65097-cp-repeated-symlink-copy branch from b91d82d to 7820495 Compare August 20, 2026 05:10
@haramj

haramj commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants