fs: bail on permission error in recursive directory creation

When creating directories recursively, the logic should bail immediately on UV_EACCES and bubble the error to the user. PR-URL: #31505Fixes: #31481 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Author: bcoe

src/node_file.cc

@@ -1226,11 +1226,17 @@ int MKDirpSync(uv_loop_t* loop,
 int err = uv_fs_mkdir(loop, req, next_path.c_str(), mode, nullptr);
 while (true){
 switch (err){
+// Note: uv_fs_req_cleanup in terminal paths will be called by
+// ~FSReqWrapSync():
 case0:
 if (continuation_data.paths().size() == 0){
 return0;
  }
 break;
+case UV_EACCES:
+case UV_EPERM:{
+return err;
+ }
 case UV_ENOENT:{
  std::string dirname = next_path.substr(0,
  next_path.find_last_of(kPathSeparator));
@@ -1243,9 +1249,6 @@ int MKDirpSync(uv_loop_t* loop,
  }
 break;
  }
-case UV_EPERM:{
-return err;
- }
 default:
 uv_fs_req_cleanup(req);
 int orig_err = err;
@@ -1293,6 +1296,8 @@ int MKDirpAsync(uv_loop_t* loop,
 
 while (true){
 switch (err){
+// Note: uv_fs_req_cleanup in terminal paths will be called by
+// FSReqAfterScope::~FSReqAfterScope()
 case0:{
 if (req_wrap->continuation_data()->paths().size() == 0){
  req_wrap->continuation_data()->Done(0);
@@ -1303,6 +1308,11 @@ int MKDirpAsync(uv_loop_t* loop,
  }
 break;
  }
+case UV_EACCES:
+case UV_EPERM:{
+ req_wrap->continuation_data()->Done(err);
+break;
+ }
 case UV_ENOENT:{
  std::string dirname = path.substr(0,
  path.find_last_of(kPathSeparator));
@@ -1318,10 +1328,6 @@ int MKDirpAsync(uv_loop_t* loop,
  req_wrap->continuation_data()->mode(), nullptr);
 break;
  }
-case UV_EPERM:{
- req_wrap->continuation_data()->Done(err);
-break;
- }
 default:
 uv_fs_req_cleanup(req);
 // Stash err for use in the callback.

test/parallel/test-fs-mkdir-recursive-eaccess.js

@@ -0,0 +1,71 @@
+'use strict';
+
+// Test that mkdir with recursive option returns appropriate error
+// when executed on folder it does not have permission to access.
+// Ref: https://github.com/nodejs/node/issues/31481
+
+constcommon=require('../common');
+
+if(!common.isWindows&&process.getuid()===0)
+common.skip('as this test should not be run as `root`');
+
+if(common.isIBMi)
+common.skip('IBMi has a different access permission mechanism');
+
+consttmpdir=require('../common/tmpdir');
+tmpdir.refresh();
+
+constassert=require('assert');
+const{ execSync }=require('child_process');
+constfs=require('fs');
+constpath=require('path');
+
+letn=0;
+
+functionmakeDirectoryReadOnly(dir){
+letaccessErrorCode='EACCES';
+if(common.isWindows){
+accessErrorCode='EPERM';
+execSync(`icacls ${dir} /inheritance:r`);
+execSync(`icacls ${dir} /deny "everyone":W`);
+}else{
+fs.chmodSync(dir,'444');
+}
+returnaccessErrorCode;
+}
+
+functionmakeDirectoryWritable(dir){
+if(common.isWindows){
+execSync(`icacls ${dir} /grant "everyone":W`);
+}
+}
+
+// Synchronous API should return an EACCESS error with path populated.
+{
+constdir=path.join(tmpdir.path,`mkdirp_${n++}`);
+fs.mkdirSync(dir);
+constcodeExpected=makeDirectoryReadOnly(dir);
+leterr=null;
+try{
+fs.mkdirSync(path.join(dir,'/foo'),{recursive: true});
+}catch(_err){
+err=_err;
+}
+makeDirectoryWritable(dir);
+assert(err);
+assert.strictEqual(err.code,codeExpected);
+assert(err.path);
+}
+
+// Asynchronous API should return an EACCESS error with path populated.
+{
+constdir=path.join(tmpdir.path,`mkdirp_${n++}`);
+fs.mkdirSync(dir);
+constcodeExpected=makeDirectoryReadOnly(dir);
+fs.mkdir(path.join(dir,'/bar'),{recursive: true},(err)=>{
+makeDirectoryWritable(dir);
+assert(err);
+assert.strictEqual(err.code,codeExpected);
+assert(err.path);
+});
+}