fs: return Buffer from mkdtemp when prefix is a Buffer

[hamidrezaghavami]

This PR fixes an issue where passing a Buffer as the prefix to the fs.mkdtemp family of functions incorrectly returned a string, which could mangle non-UTF8 paths.

The options encoding is now safely forced to 'buffer' when a Buffer prefix is detected for both the asynchronous, synchronous, and disposable versions.

Fixes: #58795

[bakkot]

Thanks!

• Should be BufferIsBuffer not Buffer.isBuffer
• Can't mutate the options object; you'll need to do options = { ...options, encoding: buffer } or something
• Needs tests
• Possibly it should be an error if encoding is explicitly specified (and not buffer)?

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.24%. Comparing base (8fec65d) to head (3e3da50).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/fs/promises.js	50.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #64397      +/-   ##
==========================================
- Coverage   90.25%   90.24%   -0.01%     
==========================================
  Files         741      741              
  Lines      241207   241225      +18     
  Branches    45430    45442      +12     
==========================================
+ Hits       217698   217704       +6     
+ Misses      15084    15082       -2     
- Partials     8425     8439      +14     

Files with missing lines	Coverage Δ
lib/fs.js	98.36% <100.00%> (+<0.01%)	⬆️
lib/internal/fs/promises.js	92.69% <50.00%> (-0.13%)	⬇️

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

[hamidrezaghavami]

@bakkot The CI is green! Here is a quick update:

Buffer check: Switched to using the internal BufferIsBuffer.
Immutability: Fixed the code to avoid mutating the user's options object.
Regression test: Added a full test suite for the Buffer prefix (test-fs-mkdtemp-buffer.js).
Original tests: Patched a string coercion bug in the original test-fs-mkdtemp.js file that was crashing the CI.

Let me know if there is anything else I need to adjust!

[bakkot]

What happened to these tests?

[bakkot]

I'm not a maintainer but the new tests look good, thanks.

Still not sure about, should it be an error if encoding is explicitly specified and not buffer. I don't know if this comes up anywhere else; if so we could look at what the established pattern is. If there isn't an established pattern I would be inclined to throw here.

[hamidrezaghavami]

My mistake! I accidentally deleted them while fixing the string coercion bug above. Just restored them in the latest push.

[hamidrezaghavami]

@nodejs-github-bot All JavaScript changes and tests are perfectly clean locally!
However, I am getting a format-cpp and lint-js-and-md error from the recent upstream merge conflict on SECURITY.md and node_trace_buffer.h. Could a maintainer please help run the formatters for me since I don't have the local C++ tooling installed? (Also, test-internet appears to be a flaky failure).
Ready for your final review!