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

❌ Patch coverage is 65.51724% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.77%. Comparing base (da5efc4) to head (9ce510e).
⚠️ Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/fs.js	72.22%	5 Missing ⚠️
lib/internal/fs/promises.js	54.54%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61968      +/-   ##
==========================================
+ Coverage   88.84%   89.77%   +0.93%     
==========================================
  Files         674      674              
  Lines      204957   205733     +776     
  Branches    39309    39445     +136     
==========================================
+ Hits       182087   184693    +2606     
+ Misses      15088    13289    -1799     
+ Partials     7782     7751      -31     

Files with missing lines	Coverage Δ
lib/fs.js	98.05% <72.22%> (+4.71%)	⬆️
lib/internal/fs/promises.js	98.37% <54.54%> (+0.31%)	⬆️

... and 184 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?

[RajeshKumar11]

Good catch, thank you!

After getValidatedPath(), the path can be:

file:// URL inputs: fileURLToPath uses new URL(path), which normalizes .. during URL construction. They arrive as already-clean strings — our check does not even need to fire.

Buffer inputs: Our original typeof path === 'string' check was skipping these. The C++ binding (RmSync) calls ToNamespacedPath, which on Windows converts paths to \?\-prefixed extended-length form — and that form does not support .. components. So Buffer inputs with .. had the same problem as string inputs.

Fixed in the latest push: Buffer inputs are now also handled by converting to a UTF-8 string via BufferToString = uncurryThis(Buffer.prototype.toString), checking for .., and normalizing if found. The normalized string is then passed to the binding (which accepts both strings and Buffers).