src,permission: add --allow-addon flag

RafaelGSS · RafaelGSS · commit a69c7d7bc375 · 2024-01-02T15:05:24.000-03:00
PR-URL: #51183 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -103,6 +103,47 @@ If this flag is passed, the behavior can still be set to not abort through
 [`process.setUncaughtExceptionCaptureCallback()`][] (and through usage of the
 `node:domain` module that uses it).
 
+### `--allow-addons`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1.1 - Active development
+
+When using the [Permission Model][], the process will not be able to use
+native addons by default.
+Attempts to do so will throw an `ERR_DLOPEN_DISABLED` unless the
+user explicitly passes the `--allow-addons` flag when starting Node.js.
+
+Example:
+
+```cjs
+// Attempt to require an native addon
+require('nodejs-addon-example');
+```
+
+```console
+$ node --experimental-permission --allow-fs-read=* index.js
+node:internal/modules/cjs/loader:1319
+  return process.dlopen(module, path.toNamespacedPath(filename));
+                 ^
+
+Error: Cannot load native addon because loading addons is disabled.
+    at Module._extensions..node (node:internal/modules/cjs/loader:1319:18)
+    at Module.load (node:internal/modules/cjs/loader:1091:32)
+    at Module._load (node:internal/modules/cjs/loader:938:12)
+    at Module.require (node:internal/modules/cjs/loader:1115:19)
+    at require (node:internal/modules/helpers:130:18)
+    at Object.<anonymous> (/home/index.js:1:15)
+    at Module._compile (node:internal/modules/cjs/loader:1233:14)
+    at Module._extensions..js (node:internal/modules/cjs/loader:1287:10)
+    at Module.load (node:internal/modules/cjs/loader:1091:32)
+    at Module._load (node:internal/modules/cjs/loader:938:12) {
+  code: 'ERR_DLOPEN_DISABLED'
+}
+```
+
 ### `--allow-child-process`
 
 <!-- YAML
@@ -2407,6 +2448,7 @@ Node.js options that are allowed are:
 
 <!-- node-options-node start -->
 
+* `--allow-addons`
 * `--allow-child-process`
 * `--allow-fs-read`
 * `--allow-fs-write`
diff --git a/doc/api/permissions.md b/doc/api/permissions.md
@@ -506,6 +506,9 @@ Error: Access to this API has been restricted
 Allowing access to spawning a process and creating worker threads can be done
 using the [`--allow-child-process`][] and [`--allow-worker`][] respectively.
 
+To allow native addons when using permission model, use the [`--allow-addons`][]
+flag.
+
 #### Runtime API
 
 When enabling the Permission Model through the [`--experimental-permission`][]
@@ -588,6 +591,7 @@ There are constraints you need to know before using this system:
 
 [Import maps]: https://url.spec.whatwg.org/#relative-url-with-fragment-string
 [Security Policy]: https://github.com/nodejs/node/blob/main/SECURITY.md
+[`--allow-addons`]: cli.md#--allow-addons
 [`--allow-child-process`]: cli.md#--allow-child-process
 [`--allow-fs-read`]: cli.md#--allow-fs-read
 [`--allow-fs-write`]: cli.md#--allow-fs-write
diff --git a/doc/node.1 b/doc/node.1
@@ -82,6 +82,9 @@ Allow file system read access when using the permission model.
 .It Fl -allow-fs-write
 Allow file system write access when using the permission model.
 .
+.It Fl -allow-addons
+Allow using native addons when using the permission model.
+.
 .It Fl -allow-child-process
 Allow spawning process when using the permission model.
 .
diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js
@@ -615,6 +615,7 @@ function initializePermission() {
                         'ExperimentalWarning');
     const { has, deny } = require('internal/process/permission');
     const warnFlags = [
+      '--allow-addons',
       '--allow-child-process',
       '--allow-worker',
     ];
@@ -655,6 +656,7 @@ function initializePermission() {
     const availablePermissionFlags = [
       '--allow-fs-read',
       '--allow-fs-write',
+      '--allow-addons',
       '--allow-child-process',
       '--allow-worker',
     ];
diff --git a/src/env.cc b/src/env.cc
@@ -894,7 +894,9 @@ Environment::Environment(IsolateData* isolate_data,
     // The process shouldn't be able to neither
     // spawn/worker nor use addons or enable inspector
     // unless explicitly allowed by the user
-    options_->allow_native_addons = false;
+    if (!options_->allow_addons) {
+      options_->allow_native_addons = false;
+    }
     flags_ = flags_ | EnvironmentFlags::kNoCreateInspector;
     permission()->Apply({"*"}, permission::PermissionScope::kInspector);
     if (!options_->allow_child_process) {
diff --git a/src/node_options.cc b/src/node_options.cc
@@ -447,6 +447,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "allow permissions to write in the filesystem",
             &EnvironmentOptions::allow_fs_write,
             kAllowedInEnvvar);
+  AddOption("--allow-addons",
+            "allow use of addons when any permissions are set",
+            &EnvironmentOptions::allow_addons,
+            kAllowedInEnvvar);
   AddOption("--allow-child-process",
             "allow use of child process when any permissions are set",
             &EnvironmentOptions::allow_child_process,
diff --git a/src/node_options.h b/src/node_options.h
@@ -123,6 +123,7 @@ class EnvironmentOptions : public Options {
   bool experimental_permission = false;
   std::vector<std::string> allow_fs_read;
   std::vector<std::string> allow_fs_write;
+  bool allow_addons = false;
   bool allow_child_process = false;
   bool allow_worker_threads = false;
   bool experimental_repl_await = true;
diff --git a/test/parallel/test-permission-allow-addons-cli.js b/test/parallel/test-permission-allow-addons-cli.js
@@ -0,0 +1,17 @@
+// Flags: --experimental-permission --allow-addons --allow-fs-read=*
+'use strict';
+
+const common = require('../common');
+common.skipIfWorker();
+
+const { createRequire } = require('node:module');
+const assert = require('node:assert');
+const fixtures = require('../common/fixtures');
+const loadFixture = createRequire(fixtures.path('node_modules'));
+// When a permission is set by cli, the process shouldn't be able
+// to require native addons unless --allow-addons is sent
+{
+  // doesNotThrow
+  const msg = loadFixture('pkgexports/no-addons');
+  assert.strictEqual(msg, 'using native addons');
+}
diff --git a/test/parallel/test-permission-no-addons.js b/test/parallel/test-permission-no-addons.js
@@ -0,0 +1,17 @@
+// Flags: --experimental-permission --allow-fs-read=*
+'use strict';
+
+const common = require('../common');
+common.skipIfWorker();
+
+const { createRequire } = require('node:module');
+const assert = require('node:assert');
+const fixtures = require('../common/fixtures');
+const loadFixture = createRequire(fixtures.path('node_modules'));
+// When a permission is set by cli, the process shouldn't be able
+// to require native addons unless --allow-addons is sent
+{
+  // doesNotThrow
+  const msg = loadFixture('pkgexports/no-addons');
+  assert.strictEqual(msg, 'not using native addons');
+}
diff --git a/test/parallel/test-permission-warning-flags.js b/test/parallel/test-permission-warning-flags.js
@@ -5,6 +5,7 @@ const { spawnSync } = require('child_process');
 const assert = require('assert');
 
 const warnFlags = [
+  '--allow-addons',
   '--allow-child-process',
   '--allow-worker',
 ];

Original file line number	Diff line number	Diff line change
`@@ -82,6 +82,9 @@ Allow file system read access when using the permission model.`
`82`	`82`	`.It Fl -allow-fs-write`
`83`	`83`	`Allow file system write access when using the permission model.`
`84`	`84`	`.`
	`85`	`+.It Fl -allow-addons`
	`86`	`+Allow using native addons when using the permission model.`
	`87`	`+.`
`85`	`88`	`.It Fl -allow-child-process`
`86`	`89`	`Allow spawning process when using the permission model.`
`87`	`90`	`.`