lib: normalize . and .. in path before rm/rmSync

When fs.rm, fs.rmSync, or fs.promises.rm receive a path containing
'.' or '..' components (e.g. 'a/b/../.'), the sync and async
implementations behaved differently:

- rmSync (C++ binding): std::filesystem::remove_all deleted the
  directory's children, but when trying to remove the top-level
  entry the path became invalid (the '..' referred to a now-deleted
  directory), causing the parent directory to be left behind.

- promises.rm / rm (JS rimraf): rmdir(2) on a path ending in '.'
  returns EINVAL on POSIX, so the operation failed immediately
  without removing anything.

Fix by calling path.normalize() on string paths immediately after
getValidatedPath(), before the path is passed to either rimraf or
the C++ binding. This resolves '.' and '..' lexically (e.g.
'a/b/../.' becomes 'a'), giving both code paths a clean,
unambiguous path to operate on.

Fixes: #61958

Author: RajeshKumar11

lib/fs.js

@@ -149,6 +149,10 @@ const {
 
 const permission = require('internal/process/permission');
 
+// Matches a dotdot path component (e.g. `..`, `/../`, `../`) but not
+// filenames with consecutive dots like `a..b`.
+const dotdotRe = /(^|[\\/])\.\.(?:[\\/]|$)/;
+
 let fs;
 
 // Lazy loaded
@@ -1179,6 +1183,14 @@ function rm(path, options, callback) {
     options = undefined;
   }
   path = getValidatedPath(path);
+  if (typeof path === 'string') {
+    if (SideEffectFreeRegExpPrototypeExec(dotdotRe, path) !== null)
+      path = pathModule.normalize(path);
+  } else if (isArrayBufferView(path)) {
+    const str = BufferToString(path, 'utf8');
+    if (SideEffectFreeRegExpPrototypeExec(dotdotRe, str) !== null)
+      path = pathModule.normalize(str);
+  }
 
   validateRmOptions(path, options, false, (err, options) => {
     if (err) {
@@ -1202,8 +1214,17 @@ function rm(path, options, callback) {
  * @returns {void}
  */
 function rmSync(path, options) {
+  path = getValidatedPath(path);
+  if (typeof path === 'string') {
+    if (SideEffectFreeRegExpPrototypeExec(dotdotRe, path) !== null)
+      path = pathModule.normalize(path);
+  } else if (isArrayBufferView(path)) {
+    const str = BufferToString(path, 'utf8');
+    if (SideEffectFreeRegExpPrototypeExec(dotdotRe, str) !== null)
+      path = pathModule.normalize(str);
+  }
   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);
 }
 
 /**

lib/internal/fs/promises.js

@@ -17,6 +17,7 @@ const {
   Symbol,
   SymbolAsyncDispose,
   Uint8Array,
+  uncurryThis,
 } = primordials;
 
 const { fs: constants } = internalBinding('constants');
@@ -30,6 +31,7 @@ const {
 
 const binding = internalBinding('fs');
 const { Buffer } = require('buffer');
+const BufferToString = uncurryThis(Buffer.prototype.toString);
 
 const {
   AbortError,
@@ -92,6 +94,7 @@ const {
   promisify,
   isWindows,
   isMacOS,
+  SideEffectFreeRegExpPrototypeExec,
 } = require('internal/util');
 const EventEmitter = require('events');
 const { StringDecoder } = require('string_decoder');
@@ -102,6 +105,10 @@ const assert = require('internal/assert');
 
 const permission = require('internal/process/permission');
 
+// Matches a dotdot path component (e.g. `..`, `/../`, `../`) but not
+// filenames with consecutive dots like `a..b`.
+const dotdotRe = /(^|[\\/])\.\.(?:[\\/]|$)/;
+
 const kHandle = Symbol('kHandle');
 const kFd = Symbol('kFd');
 const kRefs = Symbol('kRefs');
@@ -802,6 +809,14 @@ async function ftruncate(handle, len = 0) {
 
 async function rm(path, options) {
   path = getValidatedPath(path);
+  if (typeof path === 'string') {
+    if (SideEffectFreeRegExpPrototypeExec(dotdotRe, path) !== null)
+      path = pathModule.normalize(path);
+  } else if (isArrayBufferView(path)) {
+    const str = BufferToString(path, 'utf8');
+    if (SideEffectFreeRegExpPrototypeExec(dotdotRe, str) !== null)
+      path = pathModule.normalize(str);
+  }
   options = await validateRmOptionsPromise(path, options, false);
   return lazyRimRaf()(path, options);
 }

test/parallel/test-fs-rm.js

@@ -569,3 +569,38 @@ if (isGitPresent) {
     }
   }
 }
+
+// Test that rm/rmSync normalize '.' and '..' in paths before processing.
+// Regression test for https://github.com/nodejs/node/issues/61958
+//
+// Each variant gets its own base directory to avoid async/sync races.
+// The weird path is constructed by joining components with path.sep so that
+// the '..' and '.' are preserved and not pre-normalized by path.join.
+{
+  // --- rmSync: <base>/a/b/../. should remove <base>/a entirely ---
+  const base = nextDirPath('dotdot-sync');
+  fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), common.mustNotMutateObjectDeep({ recursive: true }));
+  const weirdPath = [base, 'a', 'b', '..', '.'].join(path.sep);
+  fs.rmSync(weirdPath, common.mustNotMutateObjectDeep({ recursive: true }));
+  assert.strictEqual(fs.existsSync(path.join(base, 'a')), false);
+}
+
+{
+  // --- fs.rm (callback): same path construction ---
+  const base = nextDirPath('dotdot-cb');
+  fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), common.mustNotMutateObjectDeep({ recursive: true }));
+  const weirdPath = [base, 'a', 'b', '..', '.'].join(path.sep);
+  fs.rm(weirdPath, common.mustNotMutateObjectDeep({ recursive: true }), common.mustSucceed(() => {
+    assert.strictEqual(fs.existsSync(path.join(base, 'a')), false);
+  }));
+}
+
+{
+  // --- fs.promises.rm: same path construction ---
+  const base = nextDirPath('dotdot-prom');
+  fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), common.mustNotMutateObjectDeep({ recursive: true }));
+  const weirdPath = [base, 'a', 'b', '..', '.'].join(path.sep);
+  fs.promises.rm(weirdPath, common.mustNotMutateObjectDeep({ recursive: true })).then(common.mustCall(() => {
+    assert.strictEqual(fs.existsSync(path.join(base, 'a')), false);
+  }));
+}