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
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 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 70.73171% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (d6bbf57) to head (b91d82d).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/fs/cp/cp-sync.js 8.33% 11 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.12%   -0.02%     
==========================================
  Files         752      752              
  Lines      251864   252233     +369     
  Branches    47354    47452      +98     
==========================================
+ Hits       227021   227313     +292     
- Misses      16180    16216      +36     
- Partials     8663     8704      +41     
Files with missing lines Coverage Δ
lib/internal/fs/cp/cp.js 90.07% <100.00%> (+1.72%) ⬆️
src/node_file.cc 73.97% <94.73%> (-0.30%) ⬇️
lib/internal/fs/cp/cp-sync.js 57.14% <8.33%> (-1.47%) ⬇️

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

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