Skip to content

Commit bd6743b

Browse files
RafaelGSSaduh95
authored andcommitted
src,permission: implicit allow-fs-read to app entrypoint
This commit automatically includes in the allow-fs-read list all the app's entrypoints. `--require` and user entry point Signed-off-by: RafaelGSS <[email protected]> PR-URL: #58579 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
1 parent b9586bf commit bd6743b

File tree

8 files changed

+110
-13
lines changed

8 files changed

+110
-13
lines changed

doc/api/cli.md

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,9 @@ process.
195195
<!-- YAML
196196
added: v20.0.0
197197
changes:
198+
- version: REPLACEME
199+
pr-url: https://github.com/nodejs/node/pull/58579
200+
description: Entrypoints of your application are allowed to be read implicitly.
198201
- version: v22.13.0
199202
pr-url: https://github.com/nodejs/node/pull/56201
200203
description: Permission Model and --allow-fs flags are stable.
@@ -214,23 +217,20 @@ The valid arguments for the `--allow-fs-read` flag are:
214217

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

217-
The initializer module also needs to be allowed. Consider the following example:
220+
The initializer module and custom `--require` modules has a implicit
221+
read permission.
218222

219223
```console
220-
$ node --permission index.js
221-
222-
Error: Access to this API has been restricted
223-
at node:internal/main/run_main_module:23:47 {
224-
code: 'ERR_ACCESS_DENIED',
225-
permission: 'FileSystemRead',
226-
resource: '/Users/rafaelgss/repos/os/node/index.js'
227-
}
224+
$ node --permission -r custom-require.js -r custom-require-2.js index.js
228225
```
229226

230-
The process needs to have access to the `index.js` module:
227+
* The `custom-require.js`, `custom-require-2.js`, and `index.js` will be
228+
by default in the allowed read list.
231229

232-
```bash
233-
node --permission --allow-fs-read=/path/to/index.js index.js
230+
```js
231+
process.has('fs.read', 'index.js'); // true
232+
process.has('fs.read', 'custom-require.js'); // true
233+
process.has('fs.read', 'custom-require-2.js'); // true
234234
```
235235

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

doc/api/permissions.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,23 @@ $ node --permission --allow-fs-read=* --allow-fs-write=* index.js
102102
Hello world!
103103
```
104104

105+
By default the entrypoints of your application are included
106+
in the allowed file system read list. For example:
107+
108+
```console
109+
$ node --permission index.js
110+
```
111+
112+
* `index.js` will be included in the allowed file system read list
113+
114+
```console
115+
$ node -r /path/to/custom-require.js --permission index.js.
116+
```
117+
118+
* `/path/to/custom-require.js` will be included in the allowed file system read
119+
list.
120+
* `index.js` will be included in the allowed file system read list.
121+
105122
The valid arguments for both flags are:
106123

107124
* `*` - To allow all `FileSystemRead` or `FileSystemWrite` operations,

src/env.cc

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,25 @@ Environment::Environment(IsolateData* isolate_data,
952952
permission()->Apply(this, {"*"}, permission::PermissionScope::kWASI);
953953
}
954954

955+
// Implicit allow entrypoint to kFileSystemRead
956+
if (!options_->has_eval_string && !options_->force_repl) {
957+
std::string first_argv;
958+
if (argv_.size() > 1) {
959+
first_argv = argv_[1];
960+
}
961+
962+
// Also implicit allow preloaded modules to kFileSystemRead
963+
if (!options_->preload_cjs_modules.empty()) {
964+
for (const std::string& mod : options_->preload_cjs_modules) {
965+
options_->allow_fs_read.push_back(mod);
966+
}
967+
}
968+
969+
if (first_argv != "inspect") {
970+
options_->allow_fs_read.push_back(first_argv);
971+
}
972+
}
973+
955974
if (!options_->allow_fs_read.empty()) {
956975
permission()->Apply(this,
957976
options_->allow_fs_read,
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
const fs = require('node:fs')
2+
const path = require('node:path')
3+
const assert = require('node:assert');
4+
5+
{
6+
fs.readFileSync(__filename);
7+
console.log('Read its own contents') // Should not throw
8+
}
9+
{
10+
const simpleLoaderPath = path.join(__dirname, 'simple-loader.js');
11+
fs.readFile(simpleLoaderPath, (err) => {
12+
assert.ok(err.code, 'ERR_ACCESS_DENIED');
13+
assert.ok(err.permission, 'FileSystemRead');
14+
}); // Should throw ERR_ACCESS_DENIED
15+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
console.log('Hello world')
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Simulate a regular loading without fs operations
2+
// but with access to Node core modules
3+
require('node:fs')
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
2+
'use strict';
3+
4+
const common = require('../common');
5+
const { isMainThread } = require('worker_threads');
6+
7+
if (!isMainThread) {
8+
common.skip('This test only works on a main thread');
9+
}
10+
11+
if (!common.hasCrypto) {
12+
common.skip('no crypto');
13+
}
14+
15+
const assert = require('assert');
16+
const fixtures = require('../common/fixtures');
17+
const { spawnSync } = require('child_process');
18+
19+
const file = fixtures.path('permission', 'hello-world.js');
20+
const simpleLoader = fixtures.path('permission', 'simple-loader.js');
21+
const fsReadLoader = fixtures.path('permission', 'fs-read-loader.js');
22+
23+
[
24+
'',
25+
simpleLoader,
26+
fsReadLoader,
27+
].forEach((arg0) => {
28+
const { status, stderr } = spawnSync(
29+
process.execPath,
30+
[
31+
arg0 !== '' ? '-r' : '',
32+
arg0,
33+
'--permission',
34+
file,
35+
],
36+
);
37+
assert.strictEqual(status, 0, `${arg0} Error: ${stderr.toString()}`);
38+
});

test/parallel/test-permission-fs-read.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ const commonPath = path.join(__filename, '../../common');
3232
const { status, stderr } = spawnSync(
3333
process.execPath,
3434
[
35-
'--permission', `--allow-fs-read=${file}`, `--allow-fs-read=${commonPathWildcard}`, file,
35+
'--permission',
36+
// Do not uncomment this line
37+
// `--allow-fs-read=${file}`,
38+
`--allow-fs-read=${commonPathWildcard}`,
39+
file,
3640
],
3741
{
3842
env: {

0 commit comments

Comments
 (0)