From 2cd3f1a7960d593a3e8ee65816810e5e02c3bee0 Mon Sep 17 00:00:00 2001
From: bcoe <bencoe@google.com>
Date: Fri, 24 Jan 2020 22:12:39 -0800
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/31505
Fixes: https://github.com/nodejs/node/issues/31481
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 src/node_file.cc                              | 20 ++++--
 .../test-fs-mkdir-recursive-eaccess.js        | 71 +++++++++++++++++++
 2 files changed, 84 insertions(+), 7 deletions(-)
 create mode 100644 test/parallel/test-fs-mkdir-recursive-eaccess.js

diff --git a/src/node_file.cc b/src/node_file.cc
index f59cbf1fbb6cdf..5af55a38d5f5a5 100644
--- a/src/node_file.cc
+++ b/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():
         case 0:
           if (continuation_data.paths().size() == 0) {
             return 0;
           }
           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()
         case 0: {
           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.
diff --git a/test/parallel/test-fs-mkdir-recursive-eaccess.js b/test/parallel/test-fs-mkdir-recursive-eaccess.js
new file mode 100644
index 00000000000000..d0b3b2b950cf7b
--- /dev/null
+++ b/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
+
+const common = 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');
+
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+
+const assert = require('assert');
+const { execSync } = require('child_process');
+const fs = require('fs');
+const path = require('path');
+
+let n = 0;
+
+function makeDirectoryReadOnly(dir) {
+  let accessErrorCode = 'EACCES';
+  if (common.isWindows) {
+    accessErrorCode = 'EPERM';
+    execSync(`icacls ${dir} /inheritance:r`);
+    execSync(`icacls ${dir} /deny "everyone":W`);
+  } else {
+    fs.chmodSync(dir, '444');
+  }
+  return accessErrorCode;
+}
+
+function makeDirectoryWritable(dir) {
+  if (common.isWindows) {
+    execSync(`icacls ${dir} /grant "everyone":W`);
+  }
+}
+
+// Synchronous API should return an EACCESS error with path populated.
+{
+  const dir = path.join(tmpdir.path, `mkdirp_${n++}`);
+  fs.mkdirSync(dir);
+  const codeExpected = makeDirectoryReadOnly(dir);
+  let err = 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.
+{
+  const dir = path.join(tmpdir.path, `mkdirp_${n++}`);
+  fs.mkdirSync(dir);
+  const codeExpected = makeDirectoryReadOnly(dir);
+  fs.mkdir(path.join(dir, '/bar'), { recursive: true }, (err) => {
+    makeDirectoryWritable(dir);
+    assert(err);
+    assert.strictEqual(err.code, codeExpected);
+    assert(err.path);
+  });
+}