Skip to content

Commit d6ea36a

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 <rafael.nunu@hotmail.com> PR-URL: #58579 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
1 parent 8a1fe7b commit d6ea36a

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
@@ -194,6 +194,9 @@ process.
194194
<!-- YAML
195195
added: v20.0.0
196196
changes:
197+
- version: REPLACEME
198+
pr-url: https://github.com/nodejs/node/pull/58579
199+
description: Entrypoints of your application are allowed to be read implicitly.
197200
- version:
198201
- v23.5.0
199202
- v22.13.0
@@ -215,23 +218,20 @@ The valid arguments for the `--allow-fs-read` flag are:
215218

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

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

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

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

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

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

doc/api/permissions.md

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

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

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

src/env.cc

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

931+
// Implicit allow entrypoint to kFileSystemRead
932+
if (!options_->has_eval_string && !options_->force_repl) {
933+
std::string first_argv;
934+
if (argv_.size() > 1) {
935+
first_argv = argv_[1];
936+
}
937+
938+
// Also implicit allow preloaded modules to kFileSystemRead
939+
if (!options_->preload_cjs_modules.empty()) {
940+
for (const std::string& mod : options_->preload_cjs_modules) {
941+
options_->allow_fs_read.push_back(mod);
942+
}
943+
}
944+
945+
if (first_argv != "inspect") {
946+
options_->allow_fs_read.push_back(first_argv);
947+
}
948+
}
949+
931950
if (!options_->allow_fs_read.empty()) {
932951
permission()->Apply(this,
933952
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)