diff --git a/lib/fs.js b/lib/fs.js index da6b1d341c5d..638641ec5786 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -99,7 +99,6 @@ const { kReadFileBufferLength, kMaxUserId, }, - collectRecursiveReaddirResult, copyObject, Dirent, getDirents, @@ -1743,9 +1742,7 @@ function mkdirSync(path, options) { /** * Reads the entire contents of the `basePath` directory. This function does * not validate `basePath` as a directory. It is passed directly to - * `binding.readdirRecursive`, or to `binding.readdir` for each directory when - * the permission model is enabled, since that checks every directory on the - * main thread where the native walk can not. + * `binding.readdirRecursive`. * @param {string | Buffer} basePath * @param {{ encoding: string, withFileTypes: boolean }} options * @param {( @@ -1755,10 +1752,6 @@ function mkdirSync(path, options) { * @returns {void} */ function readdirRecursive(basePath, options, callback) { - if (permission.isEnabled()) { - readdirRecursiveWithPermissionModel(basePath, options, callback); - return; - } const withFileTypes = !!options.withFileTypes; const req = new FSReqCallback(); req.oncomplete = (err, result) => { @@ -1771,51 +1764,6 @@ function readdirRecursive(basePath, options, callback) { binding.readdirRecursive(basePath, options.encoding, withFileTypes, req); } -function readdirRecursiveWithPermissionModel(basePath, options, callback) { - const { encoding } = options; - const context = { - withFileTypes: !!options.withFileTypes, - results: [], - dirs: [basePath], - prefixes: [''], - }; - - let i = 0; - - /** - * Reads one directory from `context.dirs` and then moves on to the next - * one, or calls back once none are left. - * @param {string} path - * @param {string} prefix path of this directory relative to `basePath` - */ - function read(path, prefix) { - const req = new FSReqCallback(); - req.oncomplete = (err, result) => { - if (err) { - callback(err); - return; - } - - try { - collectRecursiveReaddirResult(path, prefix, result, context); - } catch (err) { - callback(err); - return; - } - - if (i < context.dirs.length) { - read(context.dirs[i], context.prefixes[i++]); - } else { - callback(null, context.results); - } - }; - - binding.readdir(path, encoding, true, req); - } - - read(context.dirs[i], context.prefixes[i++]); -} - /** * Synchronously reads the entire contents of the `basePath` directory, see * `readdirRecursive`. @@ -1825,20 +1773,6 @@ function readdirRecursiveWithPermissionModel(basePath, options, callback) { */ function readdirSyncRecursive(basePath, options) { const withFileTypes = !!options.withFileTypes; - if (permission.isEnabled()) { - const context = { - withFileTypes, - results: [], - dirs: [basePath], - prefixes: [''], - }; - for (let i = 0; i < context.dirs.length; i++) { - const dir = context.dirs[i]; - const result = binding.readdir(dir, options.encoding, true); - collectRecursiveReaddirResult(dir, context.prefixes[i], result, context); - } - return context.results; - } const result = binding.readdirRecursive(basePath, options.encoding, withFileTypes); return result !== undefined && withFileTypes ? getRecursiveDirents(basePath, result) : result; } diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index bb54d2fc596e..29022d0f45d0 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -62,7 +62,6 @@ const { kReadFileUnknownBufferLength, kWriteFileMaxChunkSize, }, - collectRecursiveReaddirResult, copyObject, getDirents, getRecursiveDirents, @@ -1686,9 +1685,6 @@ async function mkdir(path, options) { } async function readdirRecursive(originalPath, options) { - if (permission.isEnabled()) { - return readdirRecursiveWithPermissionModel(originalPath, options); - } const withFileTypes = !!options.withFileTypes; const result = await PromisePrototypeThen( binding.readdirRecursive( @@ -1703,27 +1699,6 @@ async function readdirRecursive(originalPath, options) { return withFileTypes ? getRecursiveDirents(originalPath, result) : result; } -// TODO: native? -async function readdirRecursiveWithPermissionModel(basePath, options) { - const { encoding } = options; - const context = { - withFileTypes: !!options.withFileTypes, - results: [], - dirs: [basePath], - prefixes: [''], - }; - for (let i = 0; i < context.dirs.length; i++) { - const dir = context.dirs[i]; - const result = await PromisePrototypeThen( - binding.readdir(dir, encoding, true, kUsePromises), - undefined, - handleErrorFromBinding, - ); - collectRecursiveReaddirResult(dir, context.prefixes[i], result, context); - } - return context.results; -} - async function readdir(path, options) { const h = vfsState.handlers; if (h !== null) { diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 3aadc19c4732..064207f3e6d6 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -66,7 +66,6 @@ const { validateUint32, } = require('internal/validators'); const pathModule = require('path'); -const binding = internalBinding('fs'); const { CHAR_BACKWARD_SLASH, CHAR_FORWARD_SLASH, @@ -370,40 +369,6 @@ function joinParentPath(basePath, relative) { return Buffer.concat(parts); } -/** - * Appends one directory's entries to `context.results` and the subdirectories - * still to visit to `context.dirs`. - * @param {string} dir - * @param {string} prefix - * @param {[string[], number[]]} result - * @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context - */ -function collectRecursiveReaddirResult(dir, prefix, { 0: names, 1: types }, context) { - const { length } = names; - for (let i = 0; i < length; i++) { - const name = names[i]; - const relative = prefix === '' ? name : `${prefix}${pathModule.sep}${name}`; - let isDirectory; - if (context.withFileTypes) { - const dirent = getDirent(dir, name, types[i]); - ArrayPrototypePush(context.results, dirent); - // https://github.com/nodejs/node/issues/52663 - isDirectory = dirent.isDirectory() || - (dirent.isSymbolicLink() && binding.internalModuleStat(pathModule.join(dir, name)) === 1); - } else { - ArrayPrototypePush(context.results, relative); - const type = types[i]; - isDirectory = type === UV_DIRENT_DIR || - ((type === UV_DIRENT_LINK || type === UV_DIRENT_UNKNOWN) && - binding.internalModuleStat(pathModule.join(dir, name)) === 1); - } - if (isDirectory) { - ArrayPrototypePush(context.dirs, pathModule.join(dir, name)); - ArrayPrototypePush(context.prefixes, relative); - } - } -} - function getOptions(options, defaultOptions = kEmptyObject) { if (options == null || typeof options === 'function') { return defaultOptions; @@ -1192,7 +1157,6 @@ const vfsState = { __proto__: null, handlers: null }; function setVfsHandlers(handlers) { vfsState.handlers = handlers; } module.exports = { - collectRecursiveReaddirResult, constants: { kIoMaxLength, kMaxUserId, diff --git a/src/node_file.cc b/src/node_file.cc index 0179ad12c9d8..1380943e8db0 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2327,9 +2327,7 @@ namespace { // // The tree is walked by one or more threads that share a queue of // directories, and the breadth-first order is derived from the tree -// afterwards, so it does not depend on which thread scanned what. The walk -// does not touch the Environment; lib does not use it when the permission -// model is enabled, as every directory would need a check on the main thread. +// afterwards, so it does not depend on which thread scanned what. // Maps an st_mode to the uv_dirent_type_t that uv_fs_scandir would report. uv_dirent_type_t DirentTypeFromMode(uint64_t mode) { @@ -2406,7 +2404,8 @@ struct ScannedDirectory { class RecursiveReadDir { public: - explicit RecursiveReadDir(std::string root) : root_(std::move(root)) { + RecursiveReadDir(Environment* env, std::string root) + : env_(env), root_(std::move(root)) { dirs_.push_back(std::make_unique("")); } @@ -2420,6 +2419,13 @@ class RecursiveReadDir { // 0 or a uv error code; error_path() is then the directory that failed. int error() const { return error_; } const std::string& error_path() const { return error_path_; } + // Whether the walk stopped at a directory the permission model denies + // reading, which is then error_path(). + bool access_denied() const { return access_denied_; } + + // Publishes every denial the walk met to the permission model's + // diagnostics channel. Main thread only. + void PublishDenials(); const std::vector>& dirs() const { return dirs_; @@ -2450,6 +2456,7 @@ class RecursiveReadDir { // to scan, before RunWithHelpers() starts a helper thread. static constexpr size_t kHelperThreshold = 1024; + Environment* const env_; const std::string root_; Mutex mutex_; @@ -2462,6 +2469,8 @@ class RecursiveReadDir { size_t entries_seen_ = 0; int error_ = 0; std::string error_path_; + std::vector denied_; + bool access_denied_ = false; size_t max_helpers_ = 0; std::vector helpers_; }; @@ -2472,7 +2481,16 @@ void RecursiveReadDir::RunWithHelpers(int max_helpers) { for (uv_thread_t& helper : helpers_) CHECK_EQ(uv_thread_join(&helper), 0); } +void RecursiveReadDir::PublishDenials() { + for (const std::string& path : denied_) { + env_->permission()->PublishDenied( + env_, permission::PermissionScope::kFileSystemRead, path); + } + denied_.clear(); +} + void RecursiveReadDir::Run() { + permission::Permission* const permission = env_->permission(); std::vector> subdirs; std::string path; Mutex::ScopedLock lock(mutex_); @@ -2484,16 +2502,30 @@ void RecursiveReadDir::Run() { ScannedDirectory* dir = dirs_[next_++].get(); active_++; - int r; + int r = 0; + bool denied = false; { Mutex::ScopedUnlock unlock(lock); path = root_; AppendPathComponent(&path, dir->relative); - r = Scan(path, dir, &subdirs); + // The check alone, on whichever thread this is; the denial is + // published from the main thread once the walk is over. Audit mode + // (--permission-audit) reports a denial but lets the read through. + denied = permission->enabled() && + !permission->is_granted_quiet( + env_, permission::PermissionScope::kFileSystemRead, path); + if (!denied || permission->warning_only()) r = Scan(path, dir, &subdirs); } active_--; - if (r != 0) { + if (denied) denied_.push_back(path); + if (denied && !permission->warning_only()) { + if (error_ == 0) { + error_ = UV_EACCES; + error_path_ = std::move(path); + access_denied_ = true; + } + } else if (r != 0) { if (error_ == 0) { error_ = r; error_path_ = std::move(path); @@ -2668,7 +2700,7 @@ class ReadDirRecursiveRequest { int workers) : env_(env), req_wrap_(req_wrap), - walk_(std::move(path)), + walk_(env, std::move(path)), encoding_(encoding), with_types_(with_types), pending_(workers) {} @@ -2692,6 +2724,14 @@ class ReadDirRecursiveRequest { FS_ASYNC_TRACE_END1(UV_FS_SCANDIR, req_wrap.get(), "result", walk_.error()) if (cancelled_ || !env_->can_call_into_js()) return; + walk_.PublishDenials(); + if (walk_.access_denied()) { + return permission::Permission::AsyncThrowAccessDenied( + env_, + req_wrap.get(), + permission::PermissionScope::kFileSystemRead, + walk_.error_path()); + } if (walk_.error() != 0) { return req_wrap->Reject(UVException(isolate, walk_.error(), @@ -2756,10 +2796,6 @@ static void ReadDirRecursive(const FunctionCallbackInfo& args) { bool with_types = args[2]->IsTrue(); - // Every directory would need a permission check, and only the main thread - // can do those: lib walks the tree in JS when the permission model is on. - CHECK(!env->permission()->enabled()); - if (argc > 3) { // readdirRecursive(path, encoding, withTypes, req) FSReqBase* req_wrap_async = GetReqWrap(args, 3); CHECK_NOT_NULL(req_wrap_async); @@ -2779,10 +2815,15 @@ static void ReadDirRecursive(const FunctionCallbackInfo& args) { } else { // readdirRecursive(path, encoding, withTypes) env->PrintSyncTrace(); FS_SYNC_TRACE_BEGIN(readdir); - RecursiveReadDir walk(path.ToString()); + RecursiveReadDir walk(env, path.ToString()); walk.RunWithHelpers(kReadDirRecursiveSyncHelpers); FS_SYNC_TRACE_END(readdir); + walk.PublishDenials(); + if (walk.access_denied()) { + return permission::Permission::ThrowAccessDenied( + env, permission::PermissionScope::kFileSystemRead, walk.error_path()); + } if (walk.error() != 0) { return env->ThrowUVException( walk.error(), "scandir", nullptr, walk.error_path().c_str());