Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,36 @@ When passing a single flag with a comma a warning will be displayed.

Examples can be found in the [File System Permissions][] documentation.

### `--allow-inspector`

<!-- YAML
added: REPLACEME
-->

> Stability: 1.0 - Early development

When using the [Permission Model][], the process will not be able to connect
through inspector protocol.

Attempts to do so will throw an `ERR_ACCESS_DENIED` unless the
user explicitly passes the `--allow-inspector` flag when starting Node.js.

Example:

```js
const { Session } = require('node:inspector/promises');

const session = new Session();
session.connect();
```

```console
$ node --permission index.js
Error: connect ERR_ACCESS_DENIED Access to this API has been restricted. Use --allow-inspector to manage permissions.
code: 'ERR_ACCESS_DENIED',
}
```

### `--allow-net`

<!-- YAML
Expand Down Expand Up @@ -3415,6 +3445,7 @@ one is included in the list below.
* `--allow-child-process`
* `--allow-fs-read`
* `--allow-fs-write`
* `--allow-inspector`
* `--allow-net`
* `--allow-wasi`
* `--allow-worker`
Expand Down
3 changes: 2 additions & 1 deletion doc/api/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ flag.
When starting Node.js with `--permission`,
the ability to access the file system through the `fs` module, access the network,
spawn processes, use `node:worker_threads`, use native addons, use WASI, and
enable the runtime inspector will be restricted.
enable the runtime inspector will be restricted (the listener for SIGUSR1 won't
be created).

```console
$ node --permission index.js
Expand Down
3 changes: 3 additions & 0 deletions doc/node-config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@
}
]
},
"allow-inspector": {
"type": "boolean"
},
"allow-net": {
"type": "boolean"
},
Expand Down
3 changes: 3 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ Allow using native addons when using the permission model.
.It Fl -allow-child-process
Allow spawning process when using the permission model.
.
.It Fl -allow-inspector
Allow inspector access when using the permission model.
.
.It Fl -allow-net
Allow network access when using the permission model.
.
Expand Down
1 change: 1 addition & 0 deletions lib/internal/process/permission.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ module.exports = ObjectFreeze({
'--allow-addons',
'--allow-child-process',
'--allow-net',
'--allow-inspector',
Comment on lines 42 to +43
Copy link
Member

Choose a reason for hiding this comment

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

Use alphabetical order here?

Suggested change
'--allow-net',
'--allow-inspector',
'--allow-inspector',
'--allow-net',

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do it in a follow-up PR so I don't need to re-run the tests

'--allow-wasi',
'--allow-worker',
];
Expand Down
1 change: 1 addition & 0 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,7 @@ function initializePermission() {
const warnFlags = [
'--allow-addons',
'--allow-child-process',
'--allow-inspector',
'--allow-wasi',
'--allow-worker',
];
Expand Down
6 changes: 4 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -911,8 +911,10 @@ Environment::Environment(IsolateData* isolate_data,
options_->allow_native_addons = false;
permission()->Apply(this, {"*"}, permission::PermissionScope::kAddon);
}
flags_ = flags_ | EnvironmentFlags::kNoCreateInspector;
permission()->Apply(this, {"*"}, permission::PermissionScope::kInspector);
if (!options_->allow_inspector) {
flags_ = flags_ | EnvironmentFlags::kNoCreateInspector;
permission()->Apply(this, {"*"}, permission::PermissionScope::kInspector);
}
if (!options_->allow_child_process) {
permission()->Apply(
this, {"*"}, permission::PermissionScope::kChildProcess);
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"allow use of child process when any permissions are set",
&EnvironmentOptions::allow_child_process,
kAllowedInEnvvar);
AddOption("--allow-inspector",
"allow use of inspector when any permissions are set",
&EnvironmentOptions::allow_inspector,
kAllowedInEnvvar);
AddOption("--allow-net",
"allow use of network when any permissions are set",
&EnvironmentOptions::allow_net,
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ class EnvironmentOptions : public Options {
std::vector<std::string> allow_fs_read;
std::vector<std::string> allow_fs_write;
bool allow_addons = false;
bool allow_inspector = false;
bool allow_child_process = false;
bool allow_net = false;
bool allow_wasi = false;
Expand Down
3 changes: 2 additions & 1 deletion src/permission/permission_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ namespace permission {
#define WORKER_THREADS_PERMISSIONS(V) \
V(WorkerThreads, "worker", PermissionsRoot, "--allow-worker")

#define INSPECTOR_PERMISSIONS(V) V(Inspector, "inspector", PermissionsRoot, "")
#define INSPECTOR_PERMISSIONS(V) \
V(Inspector, "inspector", PermissionsRoot, "--allow-inspector")

#define NET_PERMISSIONS(V) V(Net, "net", PermissionsRoot, "--allow-net")

Expand Down
3 changes: 3 additions & 0 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,9 @@ if (hasCrypto) {
knownGlobals.add(globalThis.SubtleCrypto);
}

const { Worker } = require('node:worker_threads');
knownGlobals.add(Worker);

function allowGlobals(...allowlist) {
for (const val of allowlist) {
knownGlobals.add(val);
Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-permission-allow-inspector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Flags: --permission --allow-fs-read=* --allow-inspector
'use strict';

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

common.skipIfInspectorDisabled();

const { Session } = require('node:inspector/promises');
const assert = require('node:assert');

const session = new Session();
session.connect();

// Guarantee WorkerImpl doesn't gets bypassed
(async () => {
await session.post('Debugger.enable');
await session.post('Runtime.enable');

globalThis.Worker = require('node:worker_threads').Worker;

const { result: { objectId } } = await session.post('Runtime.evaluate', { expression: 'Worker' });
const { internalProperties } = await session.post('Runtime.getProperties', { objectId: objectId });
const {
value: {
value: { scriptId }
}
} = internalProperties.filter((prop) => prop.name === '[[FunctionLocation]]')[0];
const { scriptSource } = await session.post('Debugger.getScriptSource', { scriptId });

const lineNumber = scriptSource.substring(0, scriptSource.indexOf('new WorkerImpl')).split('\n').length;

// Attempt to bypass it based on https://hackerone.com/reports/1962701
await session.post('Debugger.setBreakpointByUrl', {
lineNumber: lineNumber,
url: 'node:internal/worker',
columnNumber: 0,
condition: '((isInternal = true),false)'
});

assert.throws(() => {
// eslint-disable-next-line no-undef
new Worker(`
const child_process = require("node:child_process");
console.log(child_process.execSync("ls -l").toString());

console.log(require("fs").readFileSync("/etc/passwd").toString())
`, {
eval: true,
});
}, {
message: 'Access to this API has been restricted. Use --allow-worker to manage permissions.',
code: 'ERR_ACCESS_DENIED',
permission: 'WorkerThreads'
});
})().then(common.mustCall());
2 changes: 1 addition & 1 deletion test/parallel/test-permission-inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ if (!common.hasCrypto)
const session = new Session();
session.connect();
}, common.expectsError({
message: 'Access to this API has been restricted. ',
message: 'Access to this API has been restricted. Use --allow-inspector to manage permissions.',
code: 'ERR_ACCESS_DENIED',
permission: 'Inspector',
}));
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-permission-warning-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const assert = require('assert');
const warnFlags = [
'--allow-addons',
'--allow-child-process',
'--allow-inspector',
'--allow-wasi',
'--allow-worker',
];
Expand Down
Loading