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

[RajeshKumar11]

Summary

• fs.rmSync, fs.rm, and fs.promises.rm now call path.normalize() on string paths before passing them to either the C++ binding (rmSync) or the JS rimraf implementation (rm/promises.rm)
• This makes all three variants behave consistently when the path contains . or .. components

Root cause

Two different code paths handle rm in Node.js:

• rmSync (C++ — src/node_file.cc:1661): calls std::filesystem::remove_all(file_path).
  With file_path = "a/b/../.", C++ deletes the contents of a (by resolving the path through the kernel), but when it then tries to rmdir("a/b/../.") to remove the top-level entry, the b component was already deleted, so the path can't be resolved → ENOENT → treated as success → a/ is left behind.

• rm / promises.rm (JS rimraf — lib/internal/fs/rimraf.js): calls rmdir(2) on the original path first.
  On POSIX, rmdir("a/b/../.") returns EINVAL (trailing . is an invalid argument for rmdir). EINVAL is not handled by rimraf's retry logic → the operation fails immediately without removing anything.

Neither behaviour is correct, and they disagree with each other.

Fix

Apply path.normalize() to string paths right after getValidatedPath() in all three entry points:

File	Line	Change
lib/fs.js	rm()	if (typeof path === 'string') path = pathModule.normalize(path)
lib/fs.js	rmSync()	same, before both validation and binding call
lib/internal/fs/promises.js	rm()	same

path.normalize('a/b/../.') → 'a', which both rimraf and the C++ binding handle without issue. Buffer paths are left unchanged (they are an edge case and path.normalize only accepts strings).

Test

Three new test cases added to test/parallel/test-fs-rm.js:

• fs.rmSync with a ../. path removes the target directory completely
• fs.rm (callback) with the same path
• fs.promises.rm with the same path

Each case creates <base>/a/b/c/d, applies rm to <base>/a/b/../. (which should resolve to <base>/a), and asserts that <base>/a no longer exists.

Related

• Ref: isaacs/rimraf — reporter notes a matching fix can also be applied upstream

Fixes: #61958

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.77%. Comparing base (da5efc4) to head (9654815).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61968      +/-   ##
==========================================
+ Coverage   88.84%   89.77%   +0.93%     
==========================================
  Files         674      674              
  Lines      204957   205674     +717     
  Branches    39309    39428     +119     
==========================================
+ Hits       182087   184638    +2551     
+ Misses      15088    13269    -1819     
+ Partials     7782     7767      -15     

Files with missing lines	Coverage Δ
lib/fs.js	98.19% <100.00%> (+4.85%)	⬆️
lib/internal/fs/promises.js	98.73% <100.00%> (+0.67%)	⬆️

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

[isaacs]

In general I think this is the right approach. Just one question about how it handles non-string inputs.

[isaacs]

What happens if it's not a string? Does rm allow non-string values (file:// urls, buffers, etc?) If so, what happens if they correspond to a ../. type of path?