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,lib: stabilize permission model #56201

Merged
merged 3 commits into from
Dec 12, 2024
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
2 changes: 1 addition & 1 deletion benchmark/fs/readfile-permission-enabled.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const bench = common.createBenchmark(main, {
concurrent: [1, 10],
}, {
flags: [
'--experimental-permission',
'--permission',
'--allow-fs-read=*',
'--allow-fs-write=*',
'--allow-child-process',
Expand Down
2 changes: 1 addition & 1 deletion benchmark/permission/permission-processhas-fs-read.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const rootPath = path.resolve(__dirname, '../../..');

const options = {
flags: [
'--experimental-permission',
'--permission',
`--allow-fs-read=${rootPath}`,
'--allow-child-process',
'--no-warnings',
Expand Down
2 changes: 1 addition & 1 deletion benchmark/permission/permission-startup.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function spawnProcess(script, bench, state) {
function main({ count, script, nFiles, prefixPath }) {
script = path.resolve(__dirname, '../../', `${script}.js`);
const optionsWithScript = [
'--experimental-permission',
'--permission',
`--allow-fs-read=${script}`,
...mockFiles(nFiles, prefixPath).map((file) => '--allow-fs-read=' + file),
script,
Expand Down
63 changes: 37 additions & 26 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ require('nodejs-addon-example');
```

```console
$ node --experimental-permission --allow-fs-read=* index.js
$ node --permission --allow-fs-read=* index.js
node:internal/modules/cjs/loader:1319
return process.dlopen(module, path.toNamespacedPath(filename));
^
Expand Down Expand Up @@ -165,7 +165,7 @@ childProcess.spawn('node', ['-e', 'require("fs").writeFileSync("/new-file", "exa
```

```console
$ node --experimental-permission --allow-fs-read=* index.js
$ node --permission --allow-fs-read=* index.js
node:internal/child_process:388
const err = this._handle.spawn(options);
^
Expand All @@ -189,12 +189,15 @@ Error: Access to this API has been restricted
<!-- YAML
added: v20.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/56201
description: Permission Model and --allow-fs flags are stable.
- version: v20.7.0
pr-url: https://github.com/nodejs/node/pull/49047
description: Paths delimited by comma (`,`) are no longer allowed.
-->

> Stability: 1.1 - Active development
> Stability: 2 - Stable.

This flag configures file system read permissions using
the [Permission Model][].
Expand All @@ -210,7 +213,7 @@ Examples can be found in the [File System Permissions][] documentation.
The initializer module also needs to be allowed. Consider the following example:

```console
$ node --experimental-permission index.js
$ node --permission index.js

Error: Access to this API has been restricted
at node:internal/main/run_main_module:23:47 {
Expand All @@ -223,20 +226,23 @@ Error: Access to this API has been restricted
The process needs to have access to the `index.js` module:

```bash
node --experimental-permission --allow-fs-read=/path/to/index.js index.js
node --permission --allow-fs-read=/path/to/index.js index.js
```

### `--allow-fs-write`

<!-- YAML
added: v20.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/56201
description: Permission Model and --allow-fs flags are stable.
- version: v20.7.0
pr-url: https://github.com/nodejs/node/pull/49047
description: Paths delimited by comma (`,`) are no longer allowed.
-->

> Stability: 1.1 - Active development
> Stability: 2 - Stable.

This flag configures file system write permissions using
the [Permission Model][].
Expand Down Expand Up @@ -282,7 +288,7 @@ new WASI({
```

```console
$ node --experimental-permission --allow-fs-read=* index.js
$ node --permission --allow-fs-read=* index.js

Error: Access to this API has been restricted
at node:internal/main/run_main_module:30:49 {
Expand Down Expand Up @@ -313,7 +319,7 @@ new Worker(__filename);
```

```console
$ node --experimental-permission --allow-fs-read=* index.js
$ node --permission --allow-fs-read=* index.js

Error: Access to this API has been restricted
at node:internal/main/run_main_module:17:47 {
Expand Down Expand Up @@ -935,24 +941,6 @@ added:

Enable experimental support for the network inspection with Chrome DevTools.

### `--experimental-permission`

<!-- YAML
added: v20.0.0
-->

> Stability: 1.1 - Active development

Enable the Permission Model for current process. When enabled, the
following permissions are restricted:

* File System - manageable through
[`--allow-fs-read`][], [`--allow-fs-write`][] flags
* Child Process - manageable through [`--allow-child-process`][] flag
* Worker Threads - manageable through [`--allow-worker`][] flag
* WASI - manageable through [`--allow-wasi`][] flag
* Addons - manageable through [`--allow-addons`][] flag

### `--experimental-print-required-tla`

<!-- YAML
Expand Down Expand Up @@ -1783,6 +1771,28 @@ unless either the `--pending-deprecation` command-line flag, or the
are used to provide a kind of selective "early warning" mechanism that
developers may leverage to detect deprecated API usage.

### `--permission`

<!-- YAML
added: v20.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/56201
description: Permission Model is now stable.
-->

> Stability: 2 - Stable.

Enable the Permission Model for current process. When enabled, the
following permissions are restricted:

* File System - manageable through
[`--allow-fs-read`][], [`--allow-fs-write`][] flags
* Child Process - manageable through [`--allow-child-process`][] flag
* Worker Threads - manageable through [`--allow-worker`][] flag
* WASI - manageable through [`--allow-wasi`][] flag
* Addons - manageable through [`--allow-addons`][] flag

### `--preserve-symlinks`

<!-- YAML
Expand Down Expand Up @@ -3080,6 +3090,7 @@ one is included in the list below.
* `--openssl-legacy-provider`
* `--openssl-shared-config`
* `--pending-deprecation`
* `--permission`
* `--preserve-symlinks-main`
* `--preserve-symlinks`
* `--prof-process`
Expand Down
18 changes: 8 additions & 10 deletions doc/api/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,25 @@ If you find a potential security vulnerability, please refer to our

<!-- type=misc -->

> Stability: 1.1 - Active development
> Stability: 2 - Stable.

<!-- name=permission-model -->

The Node.js Permission Model is a mechanism for restricting access to specific
resources during execution.
The API exists behind a flag [`--experimental-permission`][] which when enabled,
The API exists behind a flag [`--permission`][] which when enabled,
will restrict access to all available permissions.

The available permissions are documented by the [`--experimental-permission`][]
The available permissions are documented by the [`--permission`][]
flag.

When starting Node.js with `--experimental-permission`,
When starting Node.js with `--permission`,
the ability to access the file system through the `fs` module, spawn processes,
use `node:worker_threads`, use native addons, use WASI, and enable the runtime inspector
will be restricted.

```console
$ node --experimental-permission index.js
$ node --permission index.js

Error: Access to this API has been restricted
at node:internal/main/run_main_module:23:47 {
Expand All @@ -64,7 +64,7 @@ flag. For WASI, use the [`--allow-wasi`][] flag.

#### Runtime API

When enabling the Permission Model through the [`--experimental-permission`][]
When enabling the Permission Model through the [`--permission`][]
flag a new property `permission` is added to the `process` object.
This property contains one function:

Expand All @@ -90,10 +90,8 @@ To allow access to the file system, use the [`--allow-fs-read`][] and
[`--allow-fs-write`][] flags:

```console
$ node --experimental-permission --allow-fs-read=* --allow-fs-write=* index.js
$ node --permission --allow-fs-read=* --allow-fs-write=* index.js
Hello world!
(node:19836) ExperimentalWarning: Permission is an experimental feature
(Use `node --trace-warnings ...` to show where the warning was created)
```

The valid arguments for both flags are:
Expand Down Expand Up @@ -165,5 +163,5 @@ There are constraints you need to know before using this system:
[`--allow-fs-write`]: cli.md#--allow-fs-write
[`--allow-wasi`]: cli.md#--allow-wasi
[`--allow-worker`]: cli.md#--allow-worker
[`--experimental-permission`]: cli.md#--experimental-permission
[`--permission`]: cli.md#--permission
[`permission.has()`]: process.md#processpermissionhasscope-reference
4 changes: 2 additions & 2 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -3107,7 +3107,7 @@ added: v20.0.0

* {Object}

This API is available through the [`--experimental-permission`][] flag.
This API is available through the [`--permission`][] flag.

`process.permission` is an object whose methods are used to manage permissions
for the current process. Additional documentation is available in the
Expand Down Expand Up @@ -4444,8 +4444,8 @@ cases:
[`'exit'`]: #event-exit
[`'message'`]: child_process.md#event-message
[`'uncaughtException'`]: #event-uncaughtexception
[`--experimental-permission`]: cli.md#--experimental-permission
[`--no-deprecation`]: cli.md#--no-deprecation
[`--permission`]: cli.md#--permission
[`--unhandled-rejections`]: cli.md#--unhandled-rejectionsmode
[`Buffer`]: buffer.md
[`ChildProcess.disconnect()`]: child_process.md#subprocessdisconnect
Expand Down
4 changes: 2 additions & 2 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ Specify the
.Ar module
to use as a custom module loader.
.
.It Fl -experimental-permission
Enable the experimental permission model.
.It Fl -permission
Enable the permission model.
.
.It Fl -experimental-shadow-realm
Use this flag to enable ShadowRealm support.
Expand Down
8 changes: 4 additions & 4 deletions lib/internal/process/permission.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@ const { validateString, validateBuffer } = require('internal/validators');
const { Buffer } = require('buffer');
const { isBuffer } = Buffer;

let experimentalPermission;
let _permission;

module.exports = ObjectFreeze({
__proto__: null,
isEnabled() {
if (experimentalPermission === undefined) {
if (_permission === undefined) {
const { getOptionValue } = require('internal/options');
experimentalPermission = getOptionValue('--experimental-permission');
_permission = getOptionValue('--permission');
}
return experimentalPermission;
return _permission;
},
has(scope, reference) {
validateString(scope, 'scope');
Expand Down
7 changes: 3 additions & 4 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -520,14 +520,13 @@ function initializeClusterIPC() {
}

function initializePermission() {
const experimentalPermission = getOptionValue('--experimental-permission');
if (experimentalPermission) {
const permission = getOptionValue('--permission');
if (permission) {
process.binding = function binding(_module) {
throw new ERR_ACCESS_DENIED('process.binding');
};
// Guarantee path module isn't monkey-patched to bypass permission model
ObjectFreeze(require('path'));
emitExperimentalWarning('Permission');
const { has } = require('internal/process/permission');
const warnFlags = [
'--allow-addons',
Expand Down Expand Up @@ -579,7 +578,7 @@ function initializePermission() {
ArrayPrototypeForEach(availablePermissionFlags, (flag) => {
const value = getOptionValue(flag);
if (value.length) {
throw new ERR_MISSING_OPTION('--experimental-permission');
throw new ERR_MISSING_OPTION('--permission');
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -920,7 +920,7 @@ Environment::Environment(IsolateData* isolate_data,
std::move(traced_value));
}

if (options_->experimental_permission) {
if (options_->permission) {
permission()->EnablePermissions();
// The process shouldn't be able to neither
// spawn/worker nor use addons or enable inspector
Expand Down
5 changes: 3 additions & 2 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -451,11 +451,12 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"experimental ES Module import.meta.resolve() parentURL support",
&EnvironmentOptions::experimental_import_meta_resolve,
kAllowedInEnvvar);
AddOption("--experimental-permission",
AddOption("--permission",
"enable the permission system",
&EnvironmentOptions::experimental_permission,
&EnvironmentOptions::permission,
kAllowedInEnvvar,
false);
AddAlias("--experimental-permission", "--permission");
AddOption("--allow-fs-read",
"allow permissions to read the filesystem",
&EnvironmentOptions::allow_fs_read,
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class EnvironmentOptions : public Options {
bool experimental_import_meta_resolve = false;
std::string input_type; // Value of --input-type
bool entry_is_url = false;
bool experimental_permission = false;
bool permission = false;
std::vector<std::string> allow_fs_read;
std::vector<std::string> allow_fs_write;
bool allow_addons = false;
Expand Down
2 changes: 1 addition & 1 deletion test/addons/no-addons/permission.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Flags: --experimental-permission --allow-fs-read=*
// Flags: --permission --allow-fs-read=*

'use strict';

Expand Down
6 changes: 3 additions & 3 deletions test/es-module/test-cjs-legacyMainResolve-permission.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

// Flags: --expose-internals --experimental-permission --allow-fs-read=* --allow-child-process
// Flags: --expose-internals --permission --allow-fs-read=* --allow-child-process

require('../common');

Expand Down Expand Up @@ -40,7 +40,7 @@ describe('legacyMainResolve', () => {
process.execPath,
[
'--expose-internals',
'--experimental-permission',
'--permission',
...allowReadFiles,
'-e',
`
Expand Down Expand Up @@ -98,7 +98,7 @@ describe('legacyMainResolve', () => {
process.execPath,
[
'--expose-internals',
'--experimental-permission',
'--permission',
...allowReadFiles,
'-e',
`
Expand Down
Loading
Loading