Skip to content

Commit

Permalink
process: add allowedNodeEnvironmentFlags property
Browse files Browse the repository at this point in the history
`process.allowedNodeEnvironmentFlags` provides an API to validate and
list flags as specified in `NODE_OPTIONS` from user code.

Refs: #17740
Signed-off-by: Christopher Hiller <boneskull@boneskull.com>

PR-URL: #19335
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Sam Ruby <rubys@intertwingly.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
boneskull authored and addaleax committed Aug 28, 2018
1 parent 6395b35 commit 89720bb
Show file tree
Hide file tree
Showing 7 changed files with 299 additions and 1 deletion.
51 changes: 51 additions & 0 deletions doc/api/process.md
Expand Up @@ -416,6 +416,55 @@ generate a core file.

This feature is not available in [`Worker`][] threads.

## process.allowedNodeEnvironmentFlags
<!-- YAML
added: REPLACEME
-->

* {Set}

The `process.allowedNodeEnvironmentFlags` property is a special,
read-only `Set` of flags allowable within the [`NODE_OPTIONS`][]
environment variable.

`process.allowedNodeEnvironmentFlags` extends `Set`, but overrides
`Set.prototype.has` to recognize several different possible flag
representations. `process.allowedNodeEnvironmentFlags.has()` will
return `true` in the following cases:

- Flags may omit leading single (`-`) or double (`--`) dashes; e.g.,
`inspect-brk` for `--inspect-brk`, or `r` for `-r`.
- Flags passed through to V8 (as listed in `--v8-options`) may replace
one or more *non-leading* dashes for an underscore, or vice-versa;
e.g., `--perf_basic_prof`, `--perf-basic-prof`, `--perf_basic-prof`,
etc.
- Flags may contain one or more equals (`=`) characters; all
characters after and including the first equals will be ignored;
e.g., `--stack-trace-limit=100`.
- Flags *must* be allowable within [`NODE_OPTIONS`][].

When iterating over `process.allowedNodeEnvironmentFlags`, flags will
appear only *once*; each will begin with one or more dashes. Flags
passed through to V8 will contain underscores instead of non-leading
dashes:

```js
process.allowedNodeEnvironmentFlags.forEach((flag) => {
// -r
// --inspect-brk
// --abort_on_uncaught_exception
// ...
});
```

The methods `add()`, `clear()`, and `delete()` of
`process.allowedNodeEnvironmentFlags` do nothing, and will fail
silently.

If Node.js was compiled *without* [`NODE_OPTIONS`][] support (shown in
[`process.config`][]), `process.allowedNodeEnvironmentFlags` will
contain what *would have* been allowable.

## process.arch
<!-- YAML
added: v0.5.0
Expand Down Expand Up @@ -2061,8 +2110,10 @@ cases:
[`end()`]: stream.html#stream_writable_end_chunk_encoding_callback
[`net.Server`]: net.html#net_class_net_server
[`net.Socket`]: net.html#net_class_net_socket
[`NODE_OPTIONS`]: cli.html#cli_node_options_options
[`os.constants.dlopen`]: os.html#os_dlopen_constants
[`process.argv`]: #process_process_argv
[`process.config`]: #process_process_config
[`process.execPath`]: #process_process_execpath
[`process.exit()`]: #process_process_exit_code
[`process.exitCode`]: #process_process_exitcode
Expand Down
89 changes: 89 additions & 0 deletions lib/internal/bootstrap/node.js
Expand Up @@ -194,6 +194,8 @@

perf.markMilestone(NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);

setupAllowedFlags();

// There are various modes that Node can run in. The most common two
// are running from a script and running the REPL - but there are a few
// others like the debugger or running --eval arguments. Here we decide
Expand Down Expand Up @@ -593,5 +595,92 @@
new vm.Script(source, { displayErrors: true, filename });
}

function setupAllowedFlags() {
// This builds process.allowedNodeEnvironmentFlags
// from data in the config binding

const replaceDashesRegex = /-/g;
const leadingDashesRegex = /^--?/;
const trailingValuesRegex = /=.*$/;

// Save references so user code does not interfere
const replace = Function.call.bind(String.prototype.replace);
const has = Function.call.bind(Set.prototype.has);
const test = Function.call.bind(RegExp.prototype.test);

const {
allowedV8EnvironmentFlags,
allowedNodeEnvironmentFlags
} = process.binding('config');

const trimLeadingDashes = (flag) => replace(flag, leadingDashesRegex, '');

// Save these for comparison against flags provided to
// process.allowedNodeEnvironmentFlags.has() which lack leading dashes.
// Avoid interference w/ user code by flattening `Set.prototype` into
// each object.
const [nodeFlags, v8Flags] = [
allowedNodeEnvironmentFlags, allowedV8EnvironmentFlags
].map((flags) => Object.defineProperties(
new Set(flags.map(trimLeadingDashes)),
Object.getOwnPropertyDescriptors(Set.prototype))
);

class NodeEnvironmentFlagsSet extends Set {
constructor(...args) {
super(...args);

// the super constructor consumes `add`, but
// disallow any future adds.
this.add = () => this;
}

delete() {
// noop, `Set` API compatible
return false;
}

clear() {
// noop
}

has(key) {
// This will return `true` based on various possible
// 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.
if (typeof key === 'string') {
key = replace(key, trailingValuesRegex, '');
if (test(leadingDashesRegex, key)) {
return has(this, key) ||
has(v8Flags,
replace(
replace(
key,
leadingDashesRegex,
''
),
replaceDashesRegex,
'_'
)
);
}
return has(nodeFlags, key) ||
has(v8Flags, replace(key, replaceDashesRegex, '_'));
}
return false;
}
}

Object.freeze(NodeEnvironmentFlagsSet.prototype.constructor);
Object.freeze(NodeEnvironmentFlagsSet.prototype);

process.allowedNodeEnvironmentFlags = Object.freeze(
new NodeEnvironmentFlagsSet(
allowedNodeEnvironmentFlags.concat(allowedV8EnvironmentFlags)
)
);
}

startup();
});
62 changes: 62 additions & 0 deletions src/node.cc
Expand Up @@ -587,6 +587,68 @@ 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)
Expand Down
17 changes: 17 additions & 0 deletions src/node_config.cc
Expand Up @@ -5,6 +5,7 @@

namespace node {

using v8::Array;
using v8::Boolean;
using v8::Context;
using v8::Integer;
Expand Down Expand Up @@ -132,6 +133,22 @@ static void Initialize(Local<Object> target,
READONLY_PROPERTY(debug_options_obj,
"inspectorEnabled",
Boolean::New(isolate, debug_options->inspector_enabled));

Local<Array> 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<Array> 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
Expand Down
5 changes: 5 additions & 0 deletions src/node_internals.h
Expand Up @@ -178,6 +178,11 @@ extern bool v8_initialized;

extern std::shared_ptr<PerProcessOptions> 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;

Expand Down
74 changes: 74 additions & 0 deletions test/parallel/test-process-env-allowed-flags.js
@@ -0,0 +1,74 @@
'use strict';

const assert = require('assert');
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',
'--perf_basic-prof',
'perf_basic-prof',
'perf_basic_prof',
'-r',
'r',
'--stack-trace-limit=100',
'--stack-trace-limit=-=xX_nodejs_Xx=-'
];

const badFlags = [
'--inspect_brk',
'INSPECT-BRK',
'--INSPECT-BRK',
'--r',
'-R',
'---inspect-brk',
'--cheeseburgers'
];

goodFlags.forEach((flag) => {
assert.strictEqual(
process.allowedNodeEnvironmentFlags.has(flag),
true,
`flag should be in set: ${flag}`
);
});

badFlags.forEach((flag) => {
assert.strictEqual(
process.allowedNodeEnvironmentFlags.has(flag),
false,
`flag should not be in set: ${flag}`
);
});
}

// assert all "canonical" flags begin with dash(es)
{
process.allowedNodeEnvironmentFlags.forEach((flag) => {
assert.strictEqual(/^--?[a-z8_-]+$/.test(flag), true);
});
}

// assert immutability of process.allowedNodeEnvironmentFlags
{
assert.strictEqual(Object.isFrozen(process.allowedNodeEnvironmentFlags),
true);

process.allowedNodeEnvironmentFlags.add('foo');
assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false);
process.allowedNodeEnvironmentFlags.forEach((flag) => {
assert.strictEqual(flag === 'foo', false);
});

process.allowedNodeEnvironmentFlags.clear();
assert.strictEqual(process.allowedNodeEnvironmentFlags.size > 0, true);

const size = process.allowedNodeEnvironmentFlags.size;
process.allowedNodeEnvironmentFlags.delete('-r');
assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
}
2 changes: 1 addition & 1 deletion tools/doc/type-parser.js
Expand Up @@ -16,7 +16,7 @@ const jsPrimitives = {
const jsGlobalObjectsUrl = `${jsDocPrefix}Reference/Global_Objects/`;
const jsGlobalTypes = [
'Array', 'ArrayBuffer', 'DataView', 'Date', 'Error', 'EvalError', 'Function',
'Object', 'Promise', 'RangeError', 'ReferenceError', 'RegExp',
'Object', 'Promise', 'RangeError', 'ReferenceError', 'RegExp', 'Set',
'SharedArrayBuffer', 'SyntaxError', 'TypeError', 'TypedArray', 'URIError',
'Uint8Array',
];
Expand Down

0 comments on commit 89720bb

Please sign in to comment.