FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

permission: drop process.permission.deny · nodejs/node@b441b5d · GitHub

/ node Public

Commit b441b5d

Browse files
RafaelGSS
committed
permission: drop process.permission.deny
PR-URL: #47335 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
1 parent 401d7b1 commit b441b5d

36 files changed

Lines changed: 451 additions & 797 deletions

‎benchmark/permission/permission-fs-deny.js‎

Lines changed: 0 additions & 19 deletions
This file was deleted.

benchmark/permission/permission-fs-is-granted.js renamed to benchmark/permission/permission-fs-read.js

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use strict';
22
const common = require('../common.js');
3-
const fs = require('fs/promises');
43
const path = require('path');
54

65
const configs = {
@@ -19,22 +18,11 @@ const options = {
1918

2019
const bench = common.createBenchmark(main, configs, options);
2120

22-
const recursivelyDenyFiles = async (dir) => {
23-
const files = await fs.readdir(dir, { withFileTypes: true });
24-
for (const file of files) {
25-
if (file.isDirectory()) {
26-
await recursivelyDenyFiles(path.join(dir, file.name));
27-
} else if (file.isFile()) {
28-
process.permission.deny('fs.read', [path.join(dir, file.name)]);
29-
}
30-
}
31-
};
32-
21+
// This is a naive benchmark and might not demonstrate real-world use cases.
22+
// New benchmarks will be created once the permission model config is available
23+
// through a config file.
3324
async function main(conf) {
3425
const benchmarkDir = path.join(__dirname, '../..');
35-
// Get all the benchmark files and deny access to it
36-
await recursivelyDenyFiles(benchmarkDir);
37-
3826
bench.start();
3927

4028
for (let i = 0; i < conf.n; i++) {

‎doc/api/permissions.md‎

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -492,24 +492,7 @@ using the [`--allow-child-process`][] and [`--allow-worker`][] respectively.
492492

493493
When enabling the Permission Model through the [`--experimental-permission`][]
494494
flag a new property `permission` is added to the `process` object.
495-
This property contains two functions:
496-
497-
##### `permission.deny(scope [,parameters])`
498-
499-
API call to deny permissions at runtime ([`permission.deny()`][])
500-
501-
```js
502-
process.permission.deny('fs'); // Deny permissions to ALL fs operations
503-
504-
// Deny permissions to ALL FileSystemWrite operations
505-
process.permission.deny('fs.write');
506-
// deny FileSystemWrite permissions to the protected-folder
507-
process.permission.deny('fs.write', ['/home/rafaelgss/protected-folder']);
508-
// Deny permissions to ALL FileSystemRead operations
509-
process.permission.deny('fs.read');
510-
// deny FileSystemRead permissions to the protected-folder
511-
process.permission.deny('fs.read', ['/home/rafaelgss/protected-folder']);
512-
```
495+
This property contains one function:
513496

514497
##### `permission.has(scope ,parameters)`
515498

@@ -519,10 +502,8 @@ API call to check permissions at runtime ([`permission.has()`][])
519502
process.permission.has('fs.write'); // true
520503
process.permission.has('fs.write', '/home/rafaelgss/protected-folder'); // true
521504

522-
process.permission.deny('fs.write', '/home/rafaelgss/protected-folder');
523-
524-
process.permission.has('fs.write'); // true
525-
process.permission.has('fs.write', '/home/rafaelgss/protected-folder'); // false
505+
process.permission.has('fs.read'); // true
506+
process.permission.has('fs.read', '/home/rafaelgss/protected-folder'); // false
526507
```
527508

528509
#### File System Permissions
@@ -560,39 +541,18 @@ There are constraints you need to know before using this system:
560541

561542
* Native modules are restricted by default when using the Permission Model.
562543
* Relative paths are not supported through the CLI (`--allow-fs-*`).
563-
The runtime API supports relative paths.
564544
* The model does not inherit to a child node process.
565545
* The model does not inherit to a worker thread.
566546
* When creating symlinks the target (first argument) should have read and
567547
write access.
568548
* Permission changes are not retroactively applied to existing resources.
569-
Consider the following snippet:
570-
```js
571-
const fs = require('node:fs');
572-
573-
// Open a fd
574-
const fd = fs.openSync('./README.md', 'r');
575-
// Then, deny access to all fs.read operations
576-
process.permission.deny('fs.read');
577-
// This call will NOT fail and the file will be read
578-
const data = fs.readFileSync(fd);
579-
```
580-
581-
Therefore, when possible, apply the permissions rules before any statement:
582-
583-
```js
584-
process.permission.deny('fs.read');
585-
const fd = fs.openSync('./README.md', 'r');
586-
// Error: Access to this API has been restricted
587-
```
588549

589550
[Security Policy]: https://github.com/nodejs/node/blob/main/SECURITY.md
590551
[`--allow-child-process`]: cli.md#--allow-child-process
591552
[`--allow-fs-read`]: cli.md#--allow-fs-read
592553
[`--allow-fs-write`]: cli.md#--allow-fs-write
593554
[`--allow-worker`]: cli.md#--allow-worker
594555
[`--experimental-permission`]: cli.md#--experimental-permission
595-
[`permission.deny()`]: process.md#processpermissiondenyscope-reference
596556
[`permission.has()`]: process.md#processpermissionhasscope-reference
597557
[import maps]: https://url.spec.whatwg.org/#relative-url-with-fragment-string
598558
[relative-url string]: https://url.spec.whatwg.org/#relative-url-with-fragment-string

‎doc/api/process.md‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2634,34 +2634,6 @@ This API is available through the [`--experimental-permission`][] flag.
26342634
for the current process. Additional documentation is available in the
26352635
[Permission Model][].
26362636
2637-
### `process.permission.deny(scope[, reference])`
2638-
2639-
<!-- YAML
2640-
added: REPLACEME
2641-
-->
2642-
2643-
* `scopes` {string}
2644-
* `reference` {Array}
2645-
* Returns: {boolean}
2646-
2647-
Deny permissions at runtime.
2648-
2649-
The available scopes are:
2650-
2651-
* `fs` - All File System
2652-
* `fs.read` - File System read operations
2653-
* `fs.write` - File System write operations
2654-
2655-
The reference has a meaning based on the provided scope. For example,
2656-
the reference when the scope is File System means files and folders.
2657-
2658-
```js
2659-
// Deny READ operations to the ./README.md file
2660-
process.permission.deny('fs.read', ['./README.md']);
2661-
// Deny ALL WRITE operations
2662-
process.permission.deny('fs.write');
2663-
```
2664-
26652637
### `process.permission.has(scope[, reference])`
26662638
26672639
<!-- YAML

‎lib/internal/process/permission.js‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@
22

33
const {
44
ObjectFreeze,
5-
ArrayPrototypePush,
65
} = primordials;
76

87
const permission = internalBinding('permission');
9-
const { validateString, validateArray } = require('internal/validators');
8+
const { validateString } = require('internal/validators');
109
const { isAbsolute, resolve } = require('path');
1110

1211
let experimentalPermission;
@@ -20,27 +19,6 @@ module.exports = ObjectFreeze({
2019
}
2120
return experimentalPermission;
2221
},
23-
deny(scope, references) {
24-
validateString(scope, 'scope');
25-
if (references == null) {
26-
return permission.deny(scope, references);
27-
}
28-
29-
validateArray(references, 'references');
30-
// TODO(rafaelgss): change to call fs_permission.resolve when available
31-
const normalizedParams = [];
32-
for (let i = 0; i < references.length; ++i) {
33-
if (isAbsolute(references[i])) {
34-
ArrayPrototypePush(normalizedParams, references[i]);
35-
} else {
36-
// TODO(aduh95): add support for WHATWG URLs and Uint8Arrays.
37-
ArrayPrototypePush(normalizedParams, resolve(references[i]));
38-
}
39-
}
40-
41-
return permission.deny(scope, normalizedParams);
42-
},
43-
4422
has(scope, reference) {
4523
validateString(scope, 'scope');
4624
if (reference != null) {

‎src/env.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -784,10 +784,10 @@ Environment::Environment(IsolateData* isolate_data,
784784
if (!options_->allow_fs_read.empty() || !options_->allow_fs_write.empty()) {
785785
options_->allow_native_addons = false;
786786
if (!options_->allow_child_process) {
787-
permission()->Deny(permission::PermissionScope::kChildProcess, {});
787+
permission()->Apply("*", permission::PermissionScope::kChildProcess);
788788
}
789789
if (!options_->allow_worker_threads) {
790-
permission()->Deny(permission::PermissionScope::kWorkerThreads, {});
790+
permission()->Apply("*", permission::PermissionScope::kWorkerThreads);
791791
}
792792
}
793793

‎src/permission/child_process_permission.cc‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,8 @@ namespace permission {
1010
// Currently, ChildProcess manage a single state
1111
// Once denied, it's always denied
1212
void ChildProcessPermission::Apply(const std::string& deny,
13-
PermissionScope scope) {}
14-
15-
bool ChildProcessPermission::Deny(PermissionScope perm,
16-
const std::vector<std::string>& params) {
13+
PermissionScope scope) {
1714
deny_all_ = true;
18-
return true;
1915
}
2016

2117
bool ChildProcessPermission::is_granted(PermissionScope perm,

‎src/permission/child_process_permission.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ namespace permission {
1313
class ChildProcessPermission final : public PermissionBase {
1414
public:
1515
void Apply(const std::string& deny, PermissionScope scope) override;
16-
bool Deny(PermissionScope scope,
17-
const std::vector<std::string>& params) override;
1816
bool is_granted(PermissionScope perm,
1917
const std::string_view& param = "") override;
2018

‎src/permission/fs_permission.cc‎

Lines changed: 5 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ void FreeRecursivelyNode(
4848
delete node;
4949
}
5050

51-
bool is_tree_granted(node::permission::FSPermission::RadixTree* deny_tree,
52-
node::permission::FSPermission::RadixTree* granted_tree,
51+
bool is_tree_granted(node::permission::FSPermission::RadixTree* granted_tree,
5352
const std::string_view& param) {
5453
#ifdef _WIN32
5554
// is UNC file path
@@ -60,11 +59,10 @@ bool is_tree_granted(node::permission::FSPermission::RadixTree* deny_tree,
6059
starting_pos += 4; // "UNC\"
6160
}
6261
auto normalized = param.substr(starting_pos);
63-
return !deny_tree->Lookup(normalized) &&
64-
granted_tree->Lookup(normalized, true);
62+
return granted_tree->Lookup(normalized, true);
6563
}
6664
#endif
67-
return !deny_tree->Lookup(param) && granted_tree->Lookup(param, true);
65+
return granted_tree->Lookup(param, true);
6866
}
6967

7068
} // namespace
@@ -91,40 +89,6 @@ void FSPermission::Apply(const std::string& allow, PermissionScope scope) {
9189
}
9290
}
9391

94-
bool FSPermission::Deny(PermissionScope perm,
95-
const std::vector<std::string>& params) {
96-
if (perm == PermissionScope::kFileSystem) {
97-
deny_all_in_ = true;
98-
deny_all_out_ = true;
99-
return true;
100-
}
101-
102-
bool deny_all = params.size() == 0;
103-
if (perm == PermissionScope::kFileSystemRead) {
104-
if (deny_all) deny_all_in_ = true;
105-
// when deny_all_in is already true permission.deny should be idempotent
106-
if (deny_all_in_) return true;
107-
allow_all_in_ = false;
108-
for (auto& param : params) {
109-
deny_in_fs_.Insert(WildcardIfDir(param));
110-
}
111-
return true;
112-
}
113-
114-
if (perm == PermissionScope::kFileSystemWrite) {
115-
if (deny_all) deny_all_out_ = true;
116-
// when deny_all_out is already true permission.deny should be idempotent
117-
if (deny_all_out_) return true;
118-
allow_all_out_ = false;
119-
120-
for (auto& param : params) {
121-
deny_out_fs_.Insert(WildcardIfDir(param));
122-
}
123-
return true;
124-
}
125-
return false;
126-
}
127-
12892
void FSPermission::GrantAccess(PermissionScope perm, std::string res) {
12993
const std::string path = WildcardIfDir(res);
13094
if (perm == PermissionScope::kFileSystemRead) {
@@ -144,11 +108,11 @@ bool FSPermission::is_granted(PermissionScope perm,
144108
case PermissionScope::kFileSystemRead:
145109
return !deny_all_in_ &&
146110
((param.empty() && allow_all_in_) || allow_all_in_ ||
147-
is_tree_granted(&deny_in_fs_, &granted_in_fs_, param));
111+
is_tree_granted(&granted_in_fs_, param));
148112
case PermissionScope::kFileSystemWrite:
149113
return !deny_all_out_ &&
150114
((param.empty() && allow_all_out_) || allow_all_out_ ||
151-
is_tree_granted(&deny_out_fs_, &granted_out_fs_, param));
115+
is_tree_granted(&granted_out_fs_, param));
152116
default:
153117
return false;
154118
}

‎src/permission/fs_permission.h‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ namespace permission {
1717
class FSPermission final : public PermissionBase {
1818
public:
1919
void Apply(const std::string& deny, PermissionScope scope) override;
20-
bool Deny(PermissionScope scope,
21-
const std::vector<std::string>& params) override;
2220
bool is_granted(PermissionScope perm, const std::string_view& param) override;
2321

2422
// For debugging purposes, use the gist function to print the whole tree
@@ -135,18 +133,9 @@ class FSPermission final : public PermissionBase {
135133
void GrantAccess(PermissionScope scope, std::string param);
136134
void RestrictAccess(PermissionScope scope,
137135
const std::vector<std::string>& params);
138-
// /tmp/* --grant
139-
// /tmp/dsadsa/t.js denied in runtime
140-
//
141-
// /tmp/text.txt -- grant
142-
// /tmp/text.txt -- denied in runtime
143-
//
144136
// fs granted on startup
145137
RadixTree granted_in_fs_;
146138
RadixTree granted_out_fs_;
147-
// fs denied in runtime
148-
RadixTree deny_in_fs_;
149-
RadixTree deny_out_fs_;
150139

151140
bool deny_all_in_ = true;
152141
bool deny_all_out_ = true;

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL