From ae51b1dee70eb328d017e70d1118fcdbc2918213 Mon Sep 17 00:00:00 2001 From: Rohith Pariki Date: Sat, 3 Oct 2026 06:52:27 +0530 Subject: [PATCH] fs: normalize dot segments in rm and rmSync When paths containing relative components (such as '.' or '..') are passed to fs.rm(), fs.rmSync(), or fs.promises.rm(), the different underlying implementations (native binding.rmSync vs JS rimraf) can produce divergent results or silent deletion failures because paths were used as-is without canonicalization. Add normalizePathForRm to resolve dot-segment components for both string and Buffer inputs using byte-safe latin1 preservation before reaching the underlying deletion routines. Fixes: https://github.com/nodejs/node/issues/61958 Signed-off-by: Rohith Pariki --- lib/fs.js | 28 +++++++++++- lib/internal/fs/promises.js | 14 ++++++ test/parallel/test-fs-rm.js | 86 +++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) diff --git a/lib/fs.js b/lib/fs.js index 4436fa2df6e6..5e73ba6fb585 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1537,6 +1537,29 @@ function rmdirSync(path, options) { binding.rmdir(path); } +/** + * Asynchronously removes files and + * directories (modeled on the standard POSIX `rm` utility). + * @param {string | Buffer | URL} path + * @param {{ + * force?: boolean; + * maxRetries?: number; + * recursive?: boolean; + * retryDelay?: number; + * }} [options] +function normalizePathForRm(path) { + if (typeof path === 'string') { + return pathModule.normalize(path); + } + if (isArrayBufferView(path)) { + const buf = Buffer.from(path.buffer, path.byteOffset, path.byteLength); + const str = BufferToString(buf, 'latin1'); + const normalized = pathModule.normalize(str); + return Buffer.from(normalized, 'latin1'); + } + return path; +} + /** * Asynchronously removes files and * directories (modeled on the standard POSIX `rm` utility). @@ -1560,6 +1583,7 @@ function rm(path, options, callback) { if (h !== null && vfsVoid(h.rm(path, options), callback)) return; path = getValidatedPath(path); + path = normalizePathForRm(path); validateRmOptions(path, options, false, (err, options) => { if (err) { @@ -1588,8 +1612,10 @@ function rmSync(path, options) { const result = h.rmSync(path, options); if (result !== undefined) return; } + path = getValidatedPath(path); + path = normalizePathForRm(path); const opts = validateRmOptionsSync(path, options, false); - return binding.rmSync(getValidatedPath(path), opts.maxRetries, opts.recursive, opts.retryDelay); + return binding.rmSync(path, opts.maxRetries, opts.recursive, opts.retryDelay); } /** diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index efa981c55e31..2c7ea4d21eed 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -1600,6 +1600,19 @@ async function ftruncate(handle, len = 0) { ); } +function normalizePathForRm(path) { + if (typeof path === 'string') { + return pathModule.normalize(path); + } + if (isArrayBufferView(path)) { + const buf = Buffer.from(path.buffer, path.byteOffset, path.byteLength); + const str = BufferToString(buf, 'latin1'); + const normalized = pathModule.normalize(str); + return Buffer.from(normalized, 'latin1'); + } + return path; +} + async function rm(path, options) { const h = vfsState.handlers; if (h !== null) { @@ -1607,6 +1620,7 @@ async function rm(path, options) { if (promise !== undefined) { await promise; return; } } path = getValidatedPath(path); + path = normalizePathForRm(path); options = await validateRmOptionsPromise(path, options, false); return lazyRimRaf()(path, options); } diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 80600d114b9a..d4badfd09514 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -657,3 +657,89 @@ if (isGitPresent) { // Clean up parent directory fs.rmSync(dirname, { recursive: true, force: true }); } + +// Test that rm/rmSync normalize '.' and '..' in paths before processing. +// Regression test for https://github.com/nodejs/node/issues/61958 +{ + const cases = [ + { segments: ['a', 'b', '..', '.'], target: 'a' }, + { segments: ['a', 'b', '..'], target: 'a' }, + { segments: ['a', '.'], target: 'a' }, + { segments: ['a', 'b', 'c', '.'], target: path.join('a', 'b', 'c') }, + { segments: ['a', 'b', 'c', '..', '..'], target: 'a' }, + { segments: ['a', 'b', 'c', 'd', '..', '..', '..'], target: 'a' }, + { segments: ['a', 'b', '..', '.', 'b'], target: path.join('a', 'b') }, + { segments: ['.', 'a'], target: 'a' }, + ]; + + for (const { segments, target } of cases) { + // rmSync with string path + { + const base = nextDirPath('rm-dot-sync-str'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), + common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = [base, ...segments].join(path.sep); + fs.rmSync(weirdPath, common.mustNotMutateObjectDeep({ recursive: true, force: true })); + assert.strictEqual(fs.existsSync(path.join(base, target)), false); + } + + // rmSync with Buffer path + { + const base = nextDirPath('rm-dot-sync-buf'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), + common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = Buffer.from([base, ...segments].join(path.sep)); + fs.rmSync(weirdPath, common.mustNotMutateObjectDeep({ recursive: true, force: true })); + assert.strictEqual(fs.existsSync(path.join(base, target)), false); + } + + // fs.rm (callback) with string path + { + const base = nextDirPath('rm-dot-cb-str'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), + common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = [base, ...segments].join(path.sep); + fs.rm(weirdPath, + common.mustNotMutateObjectDeep({ recursive: true, force: true }), + common.mustSucceed(() => { + assert.strictEqual(fs.existsSync(path.join(base, target)), false); + })); + } + + // fs.promises.rm with string path + { + const base = nextDirPath('rm-dot-prom-str'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), + common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = [base, ...segments].join(path.sep); + fs.promises.rm(weirdPath, + common.mustNotMutateObjectDeep({ recursive: true, force: true })) + .then(common.mustCall(() => { + assert.strictEqual(fs.existsSync(path.join(base, target)), false); + })); + } + + // fs.promises.rm with Buffer path + { + const base = nextDirPath('rm-dot-prom-buf'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), + common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = Buffer.from([base, ...segments].join(path.sep)); + fs.promises.rm(weirdPath, + common.mustNotMutateObjectDeep({ recursive: true, force: true })) + .then(common.mustCall(() => { + assert.strictEqual(fs.existsSync(path.join(base, target)), false); + })); + } + } + + // Non-UTF-8 Buffer path byte preservation test + { + const base = nextDirPath('rm-dot-buf-latin1'); + fs.mkdirSync(path.join(base, 'a', 'b'), + common.mustNotMutateObjectDeep({ recursive: true })); + const bufPath = Buffer.from([base, 'a', 'b', '..', '.'].join(path.sep)); + fs.rmSync(bufPath, common.mustNotMutateObjectDeep({ recursive: true, force: true })); + assert.strictEqual(fs.existsSync(path.join(base, 'a')), false); + } +}