From 24e4a5447197c1adaec1538b0d55732e21b11a87 Mon Sep 17 00:00:00 2001 From: Rafael Gonzaga Date: Thu, 3 Apr 2025 19:24:39 -0300 Subject: [PATCH] src,permission: make ERR_ACCESS_DENIED more descriptive This commit also adds a suggestion flag (if exists) when ERR_ACCESS_DENIED is thrown, so users don't need to jump into the documentation to see how to manage that permission error. PR-URL: https://github.com/nodejs/node/pull/57585 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- src/permission/permission.cc | 31 ++++++++++++++----- src/permission/permission_base.h | 17 +++++----- test/fixtures/permission/fs-read.js | 10 ++++++ test/fixtures/permission/fs-write.js | 9 ++++++ .../test-permission-child-process-cli.js | 1 + test/parallel/test-permission-inspector.js | 1 + test/parallel/test-permission-wasi.js | 1 + .../test-permission-worker-threads-cli.js | 1 + 8 files changed, 55 insertions(+), 16 deletions(-) diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 0580b8b7ea..774f093481 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -59,7 +59,7 @@ static void Has(const FunctionCallbackInfo& args) { } // namespace -#define V(Name, label, _) \ +#define V(Name, label, _, __) \ if (perm == PermissionScope::k##Name) return #Name; const char* Permission::PermissionToString(const PermissionScope perm) { PERMISSIONS(V) @@ -67,7 +67,7 @@ const char* Permission::PermissionToString(const PermissionScope perm) { } #undef V -#define V(Name, label, _) \ +#define V(Name, label, _, __) \ if (perm == label) return PermissionScope::k##Name; PermissionScope Permission::StringToPermission(const std::string& perm) { PERMISSIONS(V) @@ -84,32 +84,47 @@ Permission::Permission() : enabled_(false) { std::shared_ptr inspector = std::make_shared(); std::shared_ptr wasi = std::make_shared(); -#define V(Name, _, __) \ +#define V(Name, _, __, ___) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, fs)); FILESYSTEM_PERMISSIONS(V) #undef V -#define V(Name, _, __) \ +#define V(Name, _, __, ___) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, child_p)); CHILD_PROCESS_PERMISSIONS(V) #undef V -#define V(Name, _, __) \ +#define V(Name, _, __, ___) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, worker_t)); WORKER_THREADS_PERMISSIONS(V) #undef V -#define V(Name, _, __) \ +#define V(Name, _, __, ___) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, inspector)); INSPECTOR_PERMISSIONS(V) #undef V -#define V(Name, _, __) \ +#define V(Name, _, __, ___) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, wasi)); WASI_PERMISSIONS(V) #undef V } +const char* GetErrorFlagSuggestion(node::permission::PermissionScope perm) { + switch (perm) { +#define V(Name, _, __, Flag) \ + case node::permission::PermissionScope::k##Name: \ + return Flag[0] != '\0' ? "Use " Flag " to manage permissions." : ""; + PERMISSIONS(V) +#undef V + default: + return ""; + } +} + MaybeLocal CreateAccessDeniedError(Environment* env, PermissionScope perm, const std::string_view& res) { - Local err = ERR_ACCESS_DENIED(env->isolate()); + const char* suggestion = GetErrorFlagSuggestion(perm); + Local err = ERR_ACCESS_DENIED( + env->isolate(), "Access to this API has been restricted. %s", suggestion); + Local perm_string; Local resource_string; std::string_view perm_str = Permission::PermissionToString(perm); diff --git a/src/permission/permission_base.h b/src/permission/permission_base.h index d4ab33d101..8f0bae088b 100644 --- a/src/permission/permission_base.h +++ b/src/permission/permission_base.h @@ -15,18 +15,19 @@ class Environment; namespace permission { #define FILESYSTEM_PERMISSIONS(V) \ - V(FileSystem, "fs", PermissionsRoot) \ - V(FileSystemRead, "fs.read", FileSystem) \ - V(FileSystemWrite, "fs.write", FileSystem) + V(FileSystem, "fs", PermissionsRoot, "") \ + V(FileSystemRead, "fs.read", FileSystem, "--allow-fs-read") \ + V(FileSystemWrite, "fs.write", FileSystem, "--allow-fs-write") -#define CHILD_PROCESS_PERMISSIONS(V) V(ChildProcess, "child", PermissionsRoot) +#define CHILD_PROCESS_PERMISSIONS(V) \ + V(ChildProcess, "child", PermissionsRoot, "--allow-child-process") -#define WASI_PERMISSIONS(V) V(WASI, "wasi", PermissionsRoot) +#define WASI_PERMISSIONS(V) V(WASI, "wasi", PermissionsRoot, "--allow-wasi") #define WORKER_THREADS_PERMISSIONS(V) \ - V(WorkerThreads, "worker", PermissionsRoot) + V(WorkerThreads, "worker", PermissionsRoot, "--allow-worker") -#define INSPECTOR_PERMISSIONS(V) V(Inspector, "inspector", PermissionsRoot) +#define INSPECTOR_PERMISSIONS(V) V(Inspector, "inspector", PermissionsRoot, "") #define PERMISSIONS(V) \ FILESYSTEM_PERMISSIONS(V) \ @@ -35,7 +36,7 @@ namespace permission { WORKER_THREADS_PERMISSIONS(V) \ INSPECTOR_PERMISSIONS(V) -#define V(name, _, __) k##name, +#define V(name, _, __, ___) k##name, enum class PermissionScope { kPermissionsRoot = -1, PERMISSIONS(V) kPermissionsCount diff --git a/test/fixtures/permission/fs-read.js b/test/fixtures/permission/fs-read.js index fa4ea1207f..22f4c4184a 100644 --- a/test/fixtures/permission/fs-read.js +++ b/test/fixtures/permission/fs-read.js @@ -14,6 +14,16 @@ const blockedFolder = process.env.BLOCKEDFOLDER; const allowedFolder = process.env.ALLOWEDFOLDER; const regularFile = __filename; +// Guarantee the error message suggest the --allow-fs-read +{ + fs.readFile(blockedFile, common.expectsError({ + message: 'Access to this API has been restricted. Use --allow-fs-read to manage permissions.', + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })); +} + // fs.readFile { fs.readFile(blockedFile, common.expectsError({ diff --git a/test/fixtures/permission/fs-write.js b/test/fixtures/permission/fs-write.js index 83fe3d234d..590df0b658 100644 --- a/test/fixtures/permission/fs-write.js +++ b/test/fixtures/permission/fs-write.js @@ -25,6 +25,15 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; assert.ok(!process.permission.has('fs.write', blockedFile)); } +// Guarantee the error message suggest the --allow-fs-write +{ + fs.writeFile(blockedFile, 'example', common.expectsError({ + message: 'Access to this API has been restricted. Use --allow-fs-write to manage permissions.', + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + })); +} + // fs.writeFile { assert.throws(() => { diff --git a/test/parallel/test-permission-child-process-cli.js b/test/parallel/test-permission-child-process-cli.js index 7d8fbf0564..1e87aeb56e 100644 --- a/test/parallel/test-permission-child-process-cli.js +++ b/test/parallel/test-permission-child-process-cli.js @@ -26,6 +26,7 @@ if (process.argv[2] === 'child') { assert.throws(() => { childProcess.spawn(process.execPath, ['--version']); }, common.expectsError({ + message: 'Access to this API has been restricted. Use --allow-child-process to manage permissions.', code: 'ERR_ACCESS_DENIED', permission: 'ChildProcess', })); diff --git a/test/parallel/test-permission-inspector.js b/test/parallel/test-permission-inspector.js index 4b52e12abc..878adbf934 100644 --- a/test/parallel/test-permission-inspector.js +++ b/test/parallel/test-permission-inspector.js @@ -22,6 +22,7 @@ if (!common.hasCrypto) const session = new Session(); session.connect(); }, common.expectsError({ + message: 'Access to this API has been restricted. ', code: 'ERR_ACCESS_DENIED', permission: 'Inspector', })); diff --git a/test/parallel/test-permission-wasi.js b/test/parallel/test-permission-wasi.js index 01291e6855..81dc97c7a1 100644 --- a/test/parallel/test-permission-wasi.js +++ b/test/parallel/test-permission-wasi.js @@ -13,6 +13,7 @@ const { WASI } = require('wasi'); preopens: { '/': '/' }, }); }, common.expectsError({ + message: 'Access to this API has been restricted. Use --allow-wasi to manage permissions.', code: 'ERR_ACCESS_DENIED', permission: 'WASI', })); diff --git a/test/parallel/test-permission-worker-threads-cli.js b/test/parallel/test-permission-worker-threads-cli.js index cf397c2804..ac3a109bba 100644 --- a/test/parallel/test-permission-worker-threads-cli.js +++ b/test/parallel/test-permission-worker-threads-cli.js @@ -22,6 +22,7 @@ if (isMainThread) { assert.throws(() => { new Worker(__filename); }, common.expectsError({ + message: 'Access to this API has been restricted. Use --allow-worker to manage permissions.', code: 'ERR_ACCESS_DENIED', permission: 'WorkerThreads', }));