src: fix cpSyncCopyDir to dereference symlinks when dereference option is set#62402
Open
crawfordxx wants to merge 1 commit intonodejs:mainfrom
Open
src: fix cpSyncCopyDir to dereference symlinks when dereference option is set#62402crawfordxx wants to merge 1 commit intonodejs:mainfrom
crawfordxx wants to merge 1 commit intonodejs:mainfrom
Conversation
…n is set
When fs.cpSync() is called with {dereference: true}, symlinks in the source
directory should be followed and their targets copied to the destination
instead of creating new symlinks. This behavior was correctly implemented in
the JavaScript version of cpSync, but was lost when the inner copyDir loop
was migrated to C++ in CpSyncCopyDir().
The dereference parameter was captured but only used for error-condition
checks, not to decide whether to create a symlink or copy the actual content.
Fix by checking dereference before creating symlinks: when true, copy the
actual file (using copy_file) or recurse into the actual directory
(using copy_dir_contents) that the symlink points to.
Fixes: nodejs#59168
themavik
reviewed
Mar 23, 2026
themavik
left a comment
There was a problem hiding this comment.
The dereference branch in CpSyncCopyDir now copies symlink targets via copy_dir_contents / copy_file instead of recreating links — correct behavior. nit: confirm filters (should_copy_signal / dirent_filter) still apply the same when the walk hits dereferenced symlinks as for ordinary entries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
fs.cpSync()orfs.promises.cp()is called with{dereference: true},symlinks in the source directory should be followed and their targets copied
to the destination as real files/directories. This behavior was correctly
implemented in the original JavaScript version of
cpSync, but was lost whenthe inner directory-copy loop was migrated to C++ in PR #58461.
Root Cause
In
CpSyncCopyDir(src/node_file.cc), thedereferenceparameter wascaptured by the lambda but only used for error-condition checks:
At the end of the
is_symlink()branch, a new symlink was always createdat the destination regardless of whether
dereferencewas true:Fix
When
dereferenceis true and we encounter a symlink:copy_dir_contents()copy_file()When
dereferenceis false (the default), behavior is unchanged — a newsymlink is created at the destination.
Test
Existing test case in
test/parallel/test-fs-cp.mjscovers thedereferenceoption. The specific scenario from #59168 (symlink to a file outside the source
directory with
{dereference: true, recursive: true}) now produces a regularfile at the destination instead of a symlink.
Fixes: #59168