From cf470905adf93ae21eff7aeea518e7ef735880f2 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 2 Nov 2025 13:44:00 +0100 Subject: [PATCH 1/2] src: allow --disallow-code-generation-from-strings in workers Make --disallow-code-generation-from-strings a per-isolate option instead of a V8-only option, allowing it to be passed via worker execArgv. Fixes: https://github.com/nodejs/node/issues/60371 Signed-off-by: Matteo Collina --- src/api/environment.cc | 32 +++++++- src/node.h | 9 +++ src/node_contextify.cc | 2 +- src/node_internals.h | 3 +- src/node_options.cc | 2 +- src/node_options.h | 1 + src/node_worker.cc | 6 +- ...r-disallow-code-generation-from-strings.js | 76 +++++++++++++++++++ 8 files changed, 122 insertions(+), 9 deletions(-) create mode 100644 test/parallel/test-worker-disallow-code-generation-from-strings.js diff --git a/src/api/environment.cc b/src/api/environment.cc index 3b94d860de1e79..956978092b8acc 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -465,7 +465,7 @@ Environment* CreateEnvironment( CHECK(!context.IsEmpty()); Context::Scope context_scope(context); - if (InitializeContextRuntime(context).IsNothing()) { + if (InitializeContextRuntime(context, isolate_data).IsNothing()) { FreeEnvironment(env); return nullptr; } @@ -774,10 +774,16 @@ MaybeLocal GetPerContextExports(Local context, // call NewContext and so they will experience breakages. Local NewContext(Isolate* isolate, Local object_template) { + return NewContext(isolate, object_template, nullptr); +} + +Local NewContext(Isolate* isolate, + Local object_template, + IsolateData* isolate_data) { auto context = Context::New(isolate, nullptr, object_template); if (context.IsEmpty()) return context; - if (InitializeContext(context).IsNothing()) { + if (InitializeContext(context, isolate_data).IsNothing()) { return Local(); } @@ -790,7 +796,8 @@ void ProtoThrower(const FunctionCallbackInfo& info) { // This runs at runtime, regardless of whether the context // is created from a snapshot. -Maybe InitializeContextRuntime(Local context) { +Maybe InitializeContextRuntime(Local context, + IsolateData* isolate_data) { Isolate* isolate = Isolate::GetCurrent(); HandleScope handle_scope(isolate); @@ -802,6 +809,18 @@ Maybe InitializeContextRuntime(Local context) { // to the runtime flags, propagate the value to the embedder data. bool is_code_generation_from_strings_allowed = context->IsCodeGenerationFromStringsAllowed(); + + // Check if the Node.js option --disallow-code-generation-from-strings is set + // Use isolate_data if provided, otherwise fall back to per_process options + if (isolate_data != nullptr && + isolate_data->options()->disallow_code_generation_from_strings) { + is_code_generation_from_strings_allowed = false; + } else if (isolate_data == nullptr && + per_process::cli_options->per_isolate + ->disallow_code_generation_from_strings) { + is_code_generation_from_strings_allowed = false; + } + context->AllowCodeGenerationFromStrings(false); context->SetEmbedderData( ContextEmbedderIndex::kAllowCodeGenerationFromStrings, @@ -1026,11 +1045,16 @@ Maybe InitializePrimordials(Local context, // This initializes the main context (i.e. vm contexts are not included). Maybe InitializeContext(Local context) { + return InitializeContext(context, nullptr); +} + +Maybe InitializeContext(Local context, + IsolateData* isolate_data) { if (InitializeMainContextForSnapshot(context).IsNothing()) { return Nothing(); } - if (InitializeContextRuntime(context).IsNothing()) { + if (InitializeContextRuntime(context, isolate_data).IsNothing()) { return Nothing(); } return Just(true); diff --git a/src/node.h b/src/node.h index 32a9806ae84803..efed214138fc25 100644 --- a/src/node.h +++ b/src/node.h @@ -562,10 +562,19 @@ NODE_EXTERN v8::Local NewContext( v8::Local object_template = v8::Local()); +// Overload that accepts IsolateData for per-isolate options +v8::Local NewContext(v8::Isolate* isolate, + v8::Local object_template, + IsolateData* isolate_data); + // Runs Node.js-specific tweaks on an already constructed context // Return value indicates success of operation NODE_EXTERN v8::Maybe InitializeContext(v8::Local context); +// Overload that accepts IsolateData for per-isolate options +v8::Maybe InitializeContext(v8::Local context, + IsolateData* isolate_data); + // Associate the context with the given Environment. This registers the context // as known to Node.js, makes it available to the inspector. This also registers // Node.js promise hooks on the context. diff --git a/src/node_contextify.cc b/src/node_contextify.cc index f319420ae02f35..0a19dce8b99ef3 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -247,7 +247,7 @@ ContextifyContext* ContextifyContext::New(Local v8_context, // only initialized when needed because even deserializing them slows // things down significantly and they are only needed in rare occasions // in the vm contexts. - if (InitializeContextRuntime(v8_context).IsNothing()) { + if (InitializeContextRuntime(v8_context, env->isolate_data()).IsNothing()) { return {}; } diff --git a/src/node_internals.h b/src/node_internals.h index dbe36868600682..1d8117d8e97ee6 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -117,7 +117,8 @@ std::string GetHumanReadableProcessName(); v8::Maybe InitializeBaseContextForSnapshot( v8::Local context); -v8::Maybe InitializeContextRuntime(v8::Local context); +v8::Maybe InitializeContextRuntime(v8::Local context, + IsolateData* isolate_data); v8::Maybe InitializePrimordials(v8::Local context, IsolateData* isolate_data); v8::MaybeLocal InitializePrivateSymbols( diff --git a/src/node_options.cc b/src/node_options.cc index b7cb32f67cf614..d4f0688e744652 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -1290,7 +1290,7 @@ PerIsolateOptionsParser::PerIsolateOptionsParser( kAllowedInEnvvar); AddOption("--disallow-code-generation-from-strings", "disallow eval and friends", - V8Option{}, + &PerIsolateOptions::disallow_code_generation_from_strings, kAllowedInEnvvar); AddOption("--jitless", "disable runtime allocation of executable memory", diff --git a/src/node_options.h b/src/node_options.h index ab74dd31daab05..87ae12dc1824f8 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -304,6 +304,7 @@ class PerIsolateOptions : public Options { std::string max_old_space_size; int64_t stack_trace_limit = 10; std::string report_signal = "SIGUSR2"; + bool disallow_code_generation_from_strings = false; bool build_snapshot = false; std::string build_snapshot_config; inline EnvironmentOptions* get_per_env_options(); diff --git a/src/node_worker.cc b/src/node_worker.cc index edc21e7e556157..99131479921cd2 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -350,13 +350,15 @@ void Worker::Run() { SnapshotData::kNodeBaseContextIndex) .ToLocalChecked(); if (!context.IsEmpty() && - !InitializeContextRuntime(context).IsJust()) { + !InitializeContextRuntime(context, data.isolate_data_.get()) + .IsJust()) { context = Local(); } } else { Debug( this, "Worker %llu builds context from scratch\n", thread_id_.id); - context = NewContext(isolate_); + context = NewContext( + isolate_, Local(), data.isolate_data_.get()); } if (context.IsEmpty()) { // TODO(joyeecheung): maybe this should be kBootstrapFailure instead? diff --git a/test/parallel/test-worker-disallow-code-generation-from-strings.js b/test/parallel/test-worker-disallow-code-generation-from-strings.js new file mode 100644 index 00000000000000..52605b20674097 --- /dev/null +++ b/test/parallel/test-worker-disallow-code-generation-from-strings.js @@ -0,0 +1,76 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); + +// Test that --disallow-code-generation-from-strings can be passed to workers +// and properly blocks eval() and related code generation functions. + +// Test 1: Worker with --disallow-code-generation-from-strings should block eval +{ + const worker = new Worker(` + const { parentPort } = require('worker_threads'); + try { + eval('"test"'); + parentPort.postMessage({ evalBlocked: false }); + } catch (err) { + parentPort.postMessage({ evalBlocked: true, errorName: err.name }); + } + `, { + eval: true, + execArgv: ['--disallow-code-generation-from-strings'] + }); + + worker.on('message', common.mustCall((msg) => { + assert.strictEqual(msg.evalBlocked, true); + assert.strictEqual(msg.errorName, 'EvalError'); + })); + + worker.on('error', common.mustNotCall()); +} + +// Test 2: Worker without the flag should allow eval +{ + const worker = new Worker(` + const { parentPort } = require('worker_threads'); + try { + const result = eval('"test"'); + parentPort.postMessage({ evalBlocked: false, result }); + } catch (err) { + parentPort.postMessage({ evalBlocked: true }); + } + `, { + eval: true, + execArgv: [] + }); + + worker.on('message', common.mustCall((msg) => { + assert.strictEqual(msg.evalBlocked, false); + assert.strictEqual(msg.result, 'test'); + })); + + worker.on('error', common.mustNotCall()); +} + +// Test 3: Verify the flag also blocks Function constructor +{ + const worker = new Worker(` + const { parentPort } = require('worker_threads'); + try { + new Function('return 42')(); + parentPort.postMessage({ functionBlocked: false }); + } catch (err) { + parentPort.postMessage({ functionBlocked: true, errorName: err.name }); + } + `, { + eval: true, + execArgv: ['--disallow-code-generation-from-strings'] + }); + + worker.on('message', common.mustCall((msg) => { + assert.strictEqual(msg.functionBlocked, true); + assert.strictEqual(msg.errorName, 'EvalError'); + })); + + worker.on('error', common.mustNotCall()); +} From 06063c4e5f7810cf50638d0f11ce3fe5d5d0ff41 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 16 Jun 2026 10:03:15 +0000 Subject: [PATCH 2/2] fixup: address review comment on InitializeContextRuntime Do not rely on IsCodeGenerationFromStringsAllowed() since V8Option{} was removed for --disallow-code-generation-from-strings in node_options.cc. Instead, determine the value directly from the Node.js option. Refs: https://github.com/nodejs/node/pull/60549#discussion_r2485060773 PR-URL: https://github.com/nodejs/node/pull/60549 Signed-off-by: Matteo Collina --- src/api/environment.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 956978092b8acc..1db54aeaec8583 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -805,10 +805,10 @@ Maybe InitializeContextRuntime(Local context, // and ignores the ModifyCodeGenerationFromStrings callback. Set it to false // to delegate the code generation validation to // node::ModifyCodeGenerationFromStrings. - // The `IsCodeGenerationFromStringsAllowed` can be refreshed by V8 according - // to the runtime flags, propagate the value to the embedder data. - bool is_code_generation_from_strings_allowed = - context->IsCodeGenerationFromStringsAllowed(); + // The `V8Option{}` was removed in node_options.cc for this flag, so V8's + // internal flag is never set and IsCodeGenerationFromStringsAllowed() always + // returns true. Instead, determine the value directly from the Node.js option. + bool is_code_generation_from_strings_allowed = true; // Check if the Node.js option --disallow-code-generation-from-strings is set // Use isolate_data if provided, otherwise fall back to per_process options