diff --git a/lib/internal/fs/cp/cp-sync.js b/lib/internal/fs/cp/cp-sync.js index cdbec3e6796e..3bb828320d75 100644 --- a/lib/internal/fs/cp/cp-sync.js +++ b/lib/internal/fs/cp/cp-sync.js @@ -25,6 +25,7 @@ const { mkdirSync, opendirSync, readlinkSync, + realpathSync, statSync, symlinkSync, unlinkSync, @@ -53,6 +54,7 @@ function cpSyncFn(src, dest, opts) { if (!shouldCopy) return; } + opts = { ...opts, destRoot: dest }; fsBinding.cpSyncCheckPaths(src, dest, opts.dereference, opts.recursive); return getStats(src, dest, opts); @@ -71,7 +73,7 @@ function getStats(src, dest, opts) { srcStat.isBlockDevice()) { return onFile(srcStat, destStat, src, dest, opts); } else if (srcStat.isSymbolicLink()) { - return onLink(destStat, src, dest, opts.verbatimSymlinks); + return onLink(destStat, src, dest, opts.verbatimSymlinks, opts.destRoot); } // It is not possible to get here because all possible cases are handled above. @@ -198,7 +200,7 @@ function copyDir(src, dest, opts, mkDir, srcMode) { } // TODO(@anonrig): Move this function to C++. -function onLink(destStat, src, dest, verbatimSymlinks) { +function onLink(destStat, src, dest, verbatimSymlinks, destRoot) { let resolvedSrc = readlinkSync(src); if (!verbatimSymlinks && !isAbsolute(resolvedSrc)) { resolvedSrc = resolve(dirname(src), resolvedSrc); @@ -224,7 +226,13 @@ function onLink(destStat, src, dest, verbatimSymlinks) { resolvedDest = resolve(dirname(dest), resolvedDest); } - if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) { + // 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 = srcIsDir && resolvedSrc === resolvedDest && + !isSrcSubdir(realpathSync(src), realpathSync(destRoot)); + + if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest) && !sameTarget) { throw new ERR_FS_CP_EINVAL({ message: `cannot copy ${resolvedSrc} to a subdirectory of self ` + `${resolvedDest}`, @@ -237,7 +245,8 @@ function onLink(destStat, src, dest, verbatimSymlinks) { // Prevent copy if src is a subdir of dest since unlinking // dest in this case would result in removing src contents // and therefore a broken symlink would be created. - if (statSync(dest).isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc)) { + if (statSync(dest).isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc) && + !sameTarget) { throw new ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY({ message: `cannot overwrite ${resolvedDest} with ${resolvedSrc}`, path: dest, diff --git a/lib/internal/fs/cp/cp.js b/lib/internal/fs/cp/cp.js index 1ec5a2d89cac..d718708a9e19 100644 --- a/lib/internal/fs/cp/cp.js +++ b/lib/internal/fs/cp/cp.js @@ -43,6 +43,7 @@ const { mkdir, opendir, readlink, + realpath, stat, symlink, unlink, @@ -66,6 +67,7 @@ async function cpFn(src, dest, opts) { 'node is not recommended'; process.emitWarning(warning, 'TimestampPrecisionWarning'); } + opts = { ...opts, destRoot: dest }; const stats = await checkPaths(src, dest, opts); const { srcStat, destStat, skipped } = stats; if (skipped) return; @@ -385,7 +387,13 @@ async function onLink(destStat, src, dest, opts) { resolvedDest = resolve(dirname(dest), resolvedDest); } - if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) { + // 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 = srcIsDir && resolvedSrc === resolvedDest && + !isSrcSubdir(await realpath(src), await realpath(opts.destRoot)); + + if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest) && !sameTarget) { throw new ERR_FS_CP_EINVAL({ message: `cannot copy ${resolvedSrc} to a subdirectory of self ` + `${resolvedDest}`, @@ -399,7 +407,8 @@ async function onLink(destStat, src, dest, opts) { // dest in this case would result in removing src contents // and therefore a broken symlink would be created. const srcStat = await stat(src); - if (srcStat.isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc)) { + if (srcStat.isDirectory() && isSrcSubdir(resolvedDest, resolvedSrc) && + !sameTarget) { throw new ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY({ message: `cannot overwrite ${resolvedDest} with ${resolvedSrc}`, path: dest, diff --git a/src/node_file.cc b/src/node_file.cc index b624e9da41bd..fd1e62a30173 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -4643,18 +4643,23 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { } } +std::filesystem::path StripWindowsNamespace(std::filesystem::path path) { +#ifdef _WIN32 + auto wstr = path.wstring(); + if (wstr.starts_with(L"\\\\?\\")) { + return std::filesystem::path(wstr.substr(4)); + } +#endif + return path; +} + std::vector normalizePathToArray( const std::filesystem::path& path) { std::vector parts; std::error_code error; std::filesystem::path absPath = std::filesystem::absolute(path, error); if (error) absPath = path; -#ifdef _WIN32 - auto wstr = absPath.wstring(); - if (wstr.starts_with(L"\\\\?\\")) { - absPath = std::filesystem::path(wstr.substr(4)); - } -#endif + absPath = StripWindowsNamespace(absPath); for (const auto& part : absPath) { if (!part.empty()) parts.push_back(part.string()); } @@ -4838,9 +4843,9 @@ CpError CopyDirRecursive(const std::filesystem::path& src_path, std::function copy_dir_contents; - copy_dir_contents = [&options, ©_dir_contents, file_copy_opts]( - std::filesystem::path src, - std::filesystem::path dest) -> CpError { + copy_dir_contents = + [&options, ©_dir_contents, &dest_path, file_copy_opts]( + std::filesystem::path src, std::filesystem::path dest) -> CpError { std::error_code error; // Only the error_code overloads are used from here on: this runs on a // thread pool thread and exceptions are disabled. @@ -4888,6 +4893,24 @@ CpError CopyDirRecursive(const std::filesystem::path& src_path, return CpError::Std(error, dest_str); } + std::filesystem::path symlink_target_absolute; + if (options.fresh_destination) { + // As path.resolve() does: lexical only, absolute targets verbatim. + symlink_target_absolute = + symlink_target.is_absolute() + ? symlink_target + : std::filesystem::absolute(src / symlink_target, error) + .lexically_normal(); + } else { + symlink_target_absolute = std::filesystem::weakly_canonical( + std::filesystem::absolute(src / symlink_target, error), error); + } + if (error) { + return CpError::Std(error, dest_str); + } + symlink_target_absolute = + StripWindowsNamespace(symlink_target_absolute); + if (std::filesystem::exists(dest_file_path, error)) { if (std::filesystem::is_symlink(dest_file_path, error)) { auto current_dest_symlink_target = @@ -4896,9 +4919,37 @@ CpError CopyDirRecursive(const std::filesystem::path& src_path, return CpError::Std(error, dest_str); } + auto current_dest_symlink_target_absolute = + std::filesystem::weakly_canonical( + std::filesystem::absolute(dest_file_path.parent_path() / + current_dest_symlink_target, + error), + error); + if (error) { + return CpError::Std(error, dest_str); + } + current_dest_symlink_target_absolute = + StripWindowsNamespace(current_dest_symlink_target_absolute); + // Equal targets are safe unless they point to the destination + // root or one of its ancestors. + bool same_target = false; + if (symlink_target_absolute == + current_dest_symlink_target_absolute) { + auto canonical_dest = + std::filesystem::weakly_canonical(dest_path, error); + if (error) { + return CpError::Std(error, dest_str); + } + same_target = + !isInsideDir(symlink_target_absolute, canonical_dest); + } + if (!options.dereference && - std::filesystem::is_directory(symlink_target, error) && - isInsideDir(symlink_target, current_dest_symlink_target)) { + std::filesystem::is_directory(symlink_target_absolute, + error) && + isInsideDir(symlink_target_absolute, + current_dest_symlink_target_absolute) && + !same_target) { return {CpError::kEinval, 0, "cp", @@ -4912,7 +4963,9 @@ CpError CopyDirRecursive(const std::filesystem::path& src_path, // dest in this case would result in removing src contents // and therefore a broken symlink would be created. if (std::filesystem::is_directory(dest_file_path, error) && - isInsideDir(current_dest_symlink_target, symlink_target)) { + isInsideDir(current_dest_symlink_target_absolute, + symlink_target_absolute) && + !same_target) { return {CpError::kSymlinkToSubdirectory, 0, "cp", @@ -4939,27 +4992,6 @@ CpError CopyDirRecursive(const std::filesystem::path& src_path, } } } - std::filesystem::path symlink_target_absolute; - if (options.fresh_destination) { - // As path.resolve() does: lexical only, absolute targets verbatim. - symlink_target_absolute = - symlink_target.is_absolute() - ? symlink_target - : std::filesystem::absolute(src / symlink_target, error) - .lexically_normal(); - } else { - symlink_target_absolute = std::filesystem::weakly_canonical( - std::filesystem::absolute(src / symlink_target, error), error); - } - if (error) { - return CpError::Std(error, dest_str); - } -#ifdef _WIN32 - auto wstr = symlink_target_absolute.wstring(); - if (wstr.starts_with(L"\\\\?\\")) { - symlink_target_absolute = std::filesystem::path(wstr.substr(4)); - } -#endif if (dir_entry.is_directory(error)) { std::filesystem::create_directory_symlink( symlink_target_absolute, dest_file_path, error); diff --git a/test/parallel/test-fs-cp-async-symlink-repeated-copy.mjs b/test/parallel/test-fs-cp-async-symlink-repeated-copy.mjs new file mode 100644 index 000000000000..687ac74b7890 --- /dev/null +++ b/test/parallel/test-fs-cp-async-symlink-repeated-copy.mjs @@ -0,0 +1,73 @@ +// Repeated copies may replace identical links to unrelated directories or +// children of dest, but must still reject links to dest or its ancestors. +// Refs: https://github.com/nodejs/node/issues/65097 +import { mustNotMutateObjectDeep } from '../common/index.mjs'; +import { nextdir } from '../common/fs.js'; +import assert from 'node:assert'; +import { mkdirSync, readlinkSync, realpathSync, symlinkSync } from 'node:fs'; +import { cp } from 'node:fs/promises'; +import { join, relative } from 'node:path'; +import tmpdir from '../common/tmpdir.js'; + +tmpdir.refresh(); + +// The first unfiltered copy to a missing destination also exercises the +// native asynchronous directory-copy path. +for (const relativeTarget of [false, true]) { + const root = nextdir(); + const src = join(root, 'src'); + const dest = join(root, 'dest'); + const target = join(root, 'target'); + mkdirSync(src, { recursive: true }); + mkdirSync(target); + symlinkSync(relativeTarget ? '../target' : target, join(src, 'link'), 'dir'); + const opts = mustNotMutateObjectDeep({ recursive: true }); + await cp(src, dest, opts); + await cp(src, dest, opts); + assert.strictEqual(realpathSync(join(dest, 'link')), realpathSync(target)); +} + +for (const filtered of [false, true]) { + for (const aliased of [false, true]) { + for (const relativeTarget of [false, true]) { + for (const location of ['outside', 'inside', 'self', 'ancestor']) { + const root = nextdir(); + const src = join(root, 'src'); + const actual = join(root, 'actual'); + mkdirSync(src, { recursive: true }); + mkdirSync(join(actual, 'dest'), { recursive: true }); + let dest = join(actual, 'dest'); + if (aliased) { + const alias = join(root, 'alias'); + symlinkSync(actual, alias, 'junction'); + dest = join(alias, 'dest'); + } + + const target = { + outside: join(root, 'target'), + inside: join(actual, 'dest', 'sub'), + self: join(actual, 'dest'), + ancestor: actual, + }[location]; + mkdirSync(target, { recursive: true }); + const canonicalTarget = realpathSync(target); + symlinkSync(relativeTarget ? relative(src, canonicalTarget) : canonicalTarget, + join(src, 'link'), 'dir'); + const opts = mustNotMutateObjectDeep({ + recursive: true, + ...(filtered ? { filter: () => true } : {}), + }); + await cp(src, dest, opts); + const link = join(dest, 'link'); + const originalTarget = readlinkSync(link); + if (location === 'self' || location === 'ancestor') { + await assert.rejects(cp(src, dest, opts), { code: 'ERR_FS_CP_EINVAL' }); + assert.strictEqual(readlinkSync(link), originalTarget); + } else { + await cp(src, dest, opts); + } + assert.strictEqual(realpathSync(link), canonicalTarget); + } + } + } +} diff --git a/test/parallel/test-fs-cp-sync-symlink-repeated-copy.mjs b/test/parallel/test-fs-cp-sync-symlink-repeated-copy.mjs new file mode 100644 index 000000000000..61355b921d93 --- /dev/null +++ b/test/parallel/test-fs-cp-sync-symlink-repeated-copy.mjs @@ -0,0 +1,71 @@ +// Repeated copies may replace identical links to unrelated directories or +// children of dest, but must still reject links to dest or its ancestors. +// Refs: https://github.com/nodejs/node/issues/65097 +import { mustNotMutateObjectDeep } from '../common/index.mjs'; +import { nextdir } from '../common/fs.js'; +import assert from 'node:assert'; +import { cpSync, mkdirSync, readlinkSync, realpathSync, symlinkSync } from 'node:fs'; +import { join, relative } from 'node:path'; +import tmpdir from '../common/tmpdir.js'; + +tmpdir.refresh(); + +// Preserve coverage for initially missing destinations and relative targets. +for (const relativeTarget of [false, true]) { + const root = nextdir(); + const src = join(root, 'src'); + const dest = join(root, 'dest'); + const target = join(root, 'target'); + mkdirSync(src, { recursive: true }); + mkdirSync(target); + symlinkSync(relativeTarget ? '../target' : target, join(src, 'link'), 'dir'); + const opts = mustNotMutateObjectDeep({ recursive: true }); + cpSync(src, dest, opts); + cpSync(src, dest, opts); + assert.strictEqual(realpathSync(join(dest, 'link')), realpathSync(target)); +} + +for (const filtered of [false, true]) { + for (const aliased of [false, true]) { + for (const relativeTarget of [false, true]) { + for (const location of ['outside', 'inside', 'self', 'ancestor']) { + const root = nextdir(); + const src = join(root, 'src'); + const actual = join(root, 'actual'); + mkdirSync(src, { recursive: true }); + mkdirSync(join(actual, 'dest'), { recursive: true }); + let dest = join(actual, 'dest'); + if (aliased) { + const alias = join(root, 'alias'); + symlinkSync(actual, alias, 'junction'); + dest = join(alias, 'dest'); + } + + const target = { + outside: join(root, 'target'), + inside: join(actual, 'dest', 'sub'), + self: join(actual, 'dest'), + ancestor: actual, + }[location]; + mkdirSync(target, { recursive: true }); + const canonicalTarget = realpathSync(target); + symlinkSync(relativeTarget ? relative(src, canonicalTarget) : canonicalTarget, + join(src, 'link'), 'dir'); + const opts = mustNotMutateObjectDeep({ + recursive: true, + ...(filtered ? { filter: () => true } : {}), + }); + cpSync(src, dest, opts); + const link = join(dest, 'link'); + const originalTarget = readlinkSync(link); + if (location === 'self' || location === 'ancestor') { + assert.throws(() => cpSync(src, dest, opts), { code: 'ERR_FS_CP_EINVAL' }); + assert.strictEqual(readlinkSync(link), originalTarget); + } else { + cpSync(src, dest, opts); + } + assert.strictEqual(realpathSync(link), canonicalTarget); + } + } + } +}