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.

Sign up for GitHub

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

Jump to bottom

lib: add security warning on io full access #53609

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+26 −0
Open

lib: add security warning on io full access #53609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
67ccde3
lib: add security warning on io full access
LeoDog896 Jun 27, 2024
da71410
doc: more generic write operation warning
LeoDog896 Jun 29, 2024
4d24405
lib: don't check for `/` on windows
LeoDog896 Jun 29, 2024
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
8 changes: 8 additions & 0 deletions doc/api/permissions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,14 @@ There are constraints you need to know before using this system:
* Using existing file descriptors via the `node:fs` module bypasses the
Permission Model.

#### Allowing all write operations

When `--allow-fs-write=*` is permitted, it may inadvertently lead to invalidating
the permission model because of unintended file access
to files that have side effects when written to, like
service configuration files or internal file interfaces like
linux's `/proc`.

#### Limitations and Known Issues

* Symbolic links will be followed even to locations outside of the set of paths
Expand Down
18 changes: 18 additions & 0 deletions lib/internal/process/pre_execution.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const {
},
} = require('internal/v8/startup_snapshot');

const isWindows = process.platform === 'win32';

function prepareMainThreadExecution(expandArgv1 = false, initializeModules = true) {
return prepareExecution({
expandArgv1,
Expand Down Expand Up @@ -554,6 +556,22 @@ function initializePermission() {
'It could invalidate the permission model.', 'SecurityWarning');
}
}
const fsReadValue = getOptionValue('--allow-fs-read');
if (fsReadValue.length === 1 && (fsReadValue[0] === '*' || (!isWindows && fsReadValue[0] === '/'))) {
process.emitWarning(
'Granting all to --allow-fs-read leaks all sensitive info on the host machine.',
'SecurityWarning'
);
}
const fsWriteValue = getOptionValue('--allow-fs-write');
if (fsWriteValue.length === 1 && (fsWriteValue[0] === '*' || (!isWindows && fsWriteValue[0] === '/'))) {
process.emitWarning(
'Granting all to --allow-fs-write will invalidate the permission model. ' +
'Documentation can be found at ' +
'https://nodejs.org/api/permissions.html#allowing-all-write-operations',
'SecurityWarning'
);
}
const warnCommaFlags = [
'--allow-fs-read',
'--allow-fs-write',
Expand Down