diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index c053266de77c78..b9adae9ff356c7 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -620,9 +620,45 @@ const test = Function.call.bind(RegExp.prototype.test); const { - allowedV8EnvironmentFlags, - allowedNodeEnvironmentFlags - } = process.binding('config'); + getOptions, + types: { kV8Option }, + envSettings: { kAllowedInEnvironment } + } = internalBinding('options'); + const { options, aliases } = getOptions(); + + const allowedV8EnvironmentFlags = []; + const allowedNodeEnvironmentFlags = []; + for (const [name, info] of options) { + if (info.envVarSettings === kAllowedInEnvironment) { + if (info.type === kV8Option) { + allowedV8EnvironmentFlags.push(name); + } else { + allowedNodeEnvironmentFlags.push(name); + } + } + } + + for (const [ from, expansion ] of aliases) { + let isAccepted = true; + for (const to of expansion) { + if (!to.startsWith('-')) continue; + const recursiveExpansion = aliases.get(to); + if (recursiveExpansion) { + expansion.push(...recursiveExpansion); + continue; + } + isAccepted = options.get(to).envVarSettings === kAllowedInEnvironment; + if (!isAccepted) break; + } + if (isAccepted) { + let canonical = from; + if (canonical.endsWith('=')) + canonical = canonical.substr(0, canonical.length - 1); + if (canonical.endsWith(' ')) + canonical = canonical.substr(0, canonical.length - 4); + allowedNodeEnvironmentFlags.push(canonical); + } + } const trimLeadingDashes = (flag) => replace(flag, leadingDashesRegex, ''); @@ -660,6 +696,9 @@ // permutations of a flag, including present/missing leading // dash(es) and/or underscores-for-dashes in the case of V8-specific // flags. Strips any values after `=`, inclusive. + // TODO(addaleax): It might be more flexible to run the option parser + // on a dummy option set and see whether it rejects the argument or + // not. if (typeof key === 'string') { key = replace(key, trailingValuesRegex, ''); if (test(leadingDashesRegex, key)) { diff --git a/src/node.cc b/src/node.cc index 0f380efdcaf438..d38a014e42e087 100644 --- a/src/node.cc +++ b/src/node.cc @@ -597,68 +597,6 @@ const char* signo_string(int signo) { } } -// These are all flags available for use with NODE_OPTIONS. -// -// Disallowed flags: -// These flags cause Node to do things other than run scripts: -// --version / -v -// --eval / -e -// --print / -p -// --check / -c -// --interactive / -i -// --prof-process -// --v8-options -// These flags are disallowed because security: -// --preserve-symlinks -const char* const environment_flags[] = { - // Node options, sorted in `node --help` order for ease of comparison. - "--enable-fips", - "--experimental-modules", - "--experimenatl-repl-await", - "--experimental-vm-modules", - "--experimental-worker", - "--force-fips", - "--icu-data-dir", - "--inspect", - "--inspect-brk", - "--inspect-port", - "--loader", - "--napi-modules", - "--no-deprecation", - "--no-force-async-hooks-checks", - "--no-warnings", - "--openssl-config", - "--pending-deprecation", - "--redirect-warnings", - "--require", - "--throw-deprecation", - "--tls-cipher-list", - "--trace-deprecation", - "--trace-event-categories", - "--trace-event-file-pattern", - "--trace-events-enabled", - "--trace-sync-io", - "--trace-warnings", - "--track-heap-objects", - "--use-bundled-ca", - "--use-openssl-ca", - "--v8-pool-size", - "--zero-fill-buffers", - "-r" -}; - - // V8 options (define with '_', which allows '-' or '_') -const char* const v8_environment_flags[] = { - "--abort_on_uncaught_exception", - "--max_old_space_size", - "--perf_basic_prof", - "--perf_prof", - "--stack_trace_limit", -}; - -int v8_environment_flags_count = arraysize(v8_environment_flags); -int environment_flags_count = arraysize(environment_flags); - // Look up environment variable unless running as setuid root. bool SafeGetenv(const char* key, std::string* text) { #if !defined(__CloudABI__) && !defined(_WIN32) diff --git a/src/node_config.cc b/src/node_config.cc index 080d8550665ad3..e9561be9d7f1fa 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -5,7 +5,6 @@ namespace node { -using v8::Array; using v8::Boolean; using v8::Context; using v8::Integer; @@ -137,22 +136,6 @@ static void Initialize(Local target, READONLY_PROPERTY(debug_options_obj, "inspectorEnabled", Boolean::New(isolate, debug_options->inspector_enabled)); - - Local environmentFlags = Array::New(env->isolate(), - environment_flags_count); - READONLY_PROPERTY(target, "allowedNodeEnvironmentFlags", environmentFlags); - for (int i = 0; i < environment_flags_count; ++i) { - environmentFlags->Set(i, OneByteString(env->isolate(), - environment_flags[i])); - } - - Local v8EnvironmentFlags = Array::New(env->isolate(), - v8_environment_flags_count); - READONLY_PROPERTY(target, "allowedV8EnvironmentFlags", v8EnvironmentFlags); - for (int i = 0; i < v8_environment_flags_count; ++i) { - v8EnvironmentFlags->Set(i, OneByteString(env->isolate(), - v8_environment_flags[i])); - } } // InitConfig } // namespace node diff --git a/src/node_internals.h b/src/node_internals.h index 2f2473f28572ae..39be3f14a7c82e 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -180,11 +180,6 @@ extern bool v8_initialized; extern Mutex per_process_opts_mutex; extern std::shared_ptr per_process_opts; -extern const char* const environment_flags[]; -extern int environment_flags_count; -extern const char* const v8_environment_flags[]; -extern int v8_environment_flags_count; - // Forward declaration class Environment; diff --git a/test/parallel/test-process-env-allowed-flags.js b/test/parallel/test-process-env-allowed-flags.js index 39ad09a6c35b10..7313736368ce22 100644 --- a/test/parallel/test-process-env-allowed-flags.js +++ b/test/parallel/test-process-env-allowed-flags.js @@ -6,8 +6,6 @@ require('../common'); // assert legit flags are allowed, and bogus flags are disallowed { const goodFlags = [ - '--inspect-brk', - 'inspect-brk', '--perf_basic_prof', '--perf-basic-prof', 'perf-basic-prof', @@ -17,8 +15,11 @@ require('../common'); '-r', 'r', '--stack-trace-limit=100', - '--stack-trace-limit=-=xX_nodejs_Xx=-' - ]; + '--stack-trace-limit=-=xX_nodejs_Xx=-', + ].concat(process.config.variables.v8_enable_inspector ? [ + '--inspect-brk', + 'inspect-brk', + ] : []); const badFlags = [ '--inspect_brk', @@ -50,7 +51,7 @@ require('../common'); // assert all "canonical" flags begin with dash(es) { process.allowedNodeEnvironmentFlags.forEach((flag) => { - assert(/^--?[a-z8_-]+$/.test(flag), `Unexpected format for flag ${flag}`); + assert(/^--?[a-z28_-]+$/.test(flag), `Unexpected format for flag ${flag}`); }); }