Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: add buildflag to force context-aware addons #29631

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions doc/api/cli.md
Expand Up @@ -427,6 +427,13 @@ added: v6.0.0

Silence all process warnings (including deprecations).

### `--force-context-aware`
<!-- YAML
added: REPLACEME
-->

Disable loading native addons that are not [context-aware][].

### `--openssl-config=file`
<!-- YAML
added: v6.9.0
Expand Down Expand Up @@ -996,6 +1003,7 @@ Node.js options that are allowed are:
* `--experimental-report`
* `--experimental-vm-modules`
* `--experimental-wasm-modules`
* `--force-context-aware`
* `--force-fips`
* `--frozen-intrinsics`
* `--heapsnapshot-signal`
Expand Down Expand Up @@ -1228,3 +1236,4 @@ greater than `4` (its current default value). For more information, see the
[experimental ECMAScript Module]: esm.html#esm_resolve_hook
[libuv threadpool documentation]: http://docs.libuv.org/en/latest/threadpool.html
[remote code execution]: https://www.owasp.org/index.php/Code_Injection
[context-aware]: addons.html#addons_context_aware_addons
5 changes: 5 additions & 0 deletions doc/api/errors.md
Expand Up @@ -1611,6 +1611,11 @@ OpenSSL crypto support.
An attempt was made to use features that require [ICU][], but Node.js was not
compiled with ICU support.

<a id="ERR_NON_CONTEXT_AWARE_DISABLED"></a>
### ERR_NON_CONTEXT_AWARE_DISABLED

A non-context-aware native addon was loaded in a process that disallows them.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

including a link here for context-aware like in the cli.md file would be helpful


<a id="ERR_OUT_OF_RANGE"></a>
### ERR_OUT_OF_RANGE

Expand Down
8 changes: 8 additions & 0 deletions src/node_binding.cc
@@ -1,4 +1,5 @@
#include "node_binding.h"
#include "node_errors.h"
#include <atomic>
#include "env-inl.h"
#include "node_native_module_env.h"
Expand Down Expand Up @@ -463,6 +464,13 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
}

if (mp != nullptr) {
if (mp->nm_context_register_func == nullptr) {
if (env->options()->force_context_aware) {
dlib->Close();
THROW_ERR_NON_CONTEXT_AWARE_DISABLED(env);
codebytere marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
}
mp->nm_dso_handle = dlib->handle_;
dlib->SaveInGlobalHandleMap(mp);
} else {
Expand Down
3 changes: 3 additions & 0 deletions src/node_errors.h
Expand Up @@ -52,6 +52,7 @@ void PrintErrorString(const char* format, ...);
V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError) \
V(ERR_MISSING_PASSPHRASE, TypeError) \
V(ERR_MISSING_PLATFORM_FOR_WORKER, Error) \
V(ERR_NON_CONTEXT_AWARE_DISABLED, Error) \
V(ERR_MODULE_NOT_FOUND, Error) \
V(ERR_OUT_OF_RANGE, RangeError) \
V(ERR_SCRIPT_EXECUTION_INTERRUPTED, Error) \
Expand Down Expand Up @@ -96,6 +97,8 @@ void PrintErrorString(const char* format, ...);
V(ERR_MISSING_PLATFORM_FOR_WORKER, \
"The V8 platform used by this instance of Node does not support " \
"creating Workers") \
V(ERR_NON_CONTEXT_AWARE_DISABLED, \
"Loading non context-aware native modules has been disabled") \
V(ERR_SCRIPT_EXECUTION_INTERRUPTED, \
"Script execution was interrupted by `SIGINT`") \
V(ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER, \
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.cc
Expand Up @@ -393,6 +393,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"silence all process warnings",
&EnvironmentOptions::no_warnings,
kAllowedInEnvironment);
AddOption("--force-context-aware",
"disable loading non-context-aware addons",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extremely minor take it or leave it nit.. but the double negative here feels odd ... "disable ... non" ... perhaps "allow only context-aware addons" as an alternative?

&EnvironmentOptions::force_context_aware,
kAllowedInEnvironment);
AddOption("--pending-deprecation",
"emit pending deprecation warnings",
&EnvironmentOptions::pending_deprecation,
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Expand Up @@ -117,6 +117,7 @@ class EnvironmentOptions : public Options {
bool no_deprecation = false;
bool no_force_async_hooks_checks = false;
bool no_warnings = false;
bool force_context_aware = false;
bool pending_deprecation = false;
bool preserve_symlinks = false;
bool preserve_symlinks_main = false;
Expand Down
6 changes: 6 additions & 0 deletions test/addons/force-context-aware/binding.cc
@@ -0,0 +1,6 @@
#include <node.h>
#include <v8.h>

void init(v8::Local<v8::Object> exports) {}

NODE_MODULE(NODE_GYP_MODULE_NAME, init)
9 changes: 9 additions & 0 deletions test/addons/force-context-aware/binding.gyp
@@ -0,0 +1,9 @@
{
'targets': [
{
'target_name': 'binding',
'sources': [ 'binding.cc' ],
'includes': ['../common.gypi'],
}
]
}
4 changes: 4 additions & 0 deletions test/addons/force-context-aware/index.js
@@ -0,0 +1,4 @@
'use strict';
const common = require('../../common');

require(`./build/${common.buildType}/binding`);
13 changes: 13 additions & 0 deletions test/addons/force-context-aware/test.js
@@ -0,0 +1,13 @@
'use strict';
const common = require('../../common');
const childProcess = require('child_process');
const assert = require('assert');
const path = require('path');

const mod = path.join('test', 'addons', 'force-context-aware', 'index.js');

const execString = `"${process.execPath}" --force-context-aware ./${mod}`;
childProcess.exec(execString, common.mustCall((err) => {
const errMsg = 'Loading non context-aware native modules has been disabled';
assert.strictEqual(err.message.includes(errMsg), true);
}));
2 changes: 2 additions & 0 deletions test/addons/zlib-binding/test.js
@@ -1,3 +1,5 @@
// Flags: --force-context-aware

'use strict';

const common = require('../../common');
Expand Down