From b441b5dc6531cadfd64ae3ad5e20590dfffb1957 Mon Sep 17 00:00:00 2001 From: Rafael Gonzaga Date: Tue, 4 Apr 2023 14:14:04 -0300 Subject: [PATCH] permission: drop process.permission.deny PR-URL: https://github.com/nodejs/node/pull/47335 Reviewed-By: James M Snell Reviewed-By: Antoine du Hamel Reviewed-By: Beth Griggs Reviewed-By: Marco Ippolito --- benchmark/permission/permission-fs-deny.js | 19 --- ...fs-is-granted.js => permission-fs-read.js} | 18 +-- doc/api/permissions.md | 46 +----- doc/api/process.md | 28 ---- lib/internal/process/permission.js | 24 +--- src/env.cc | 4 +- src/permission/child_process_permission.cc | 6 +- src/permission/child_process_permission.h | 2 - src/permission/fs_permission.cc | 46 +----- src/permission/fs_permission.h | 11 -- src/permission/permission.cc | 49 ------- src/permission/permission.h | 2 - src/permission/permission_base.h | 2 - src/permission/worker_permission.cc | 6 +- src/permission/worker_permission.h | 2 - .../permission/fs-read.js} | 103 ++------------ .../permission/fs-symlink-target-write.js} | 31 ++--- .../permission/fs-symlink.js} | 29 ++-- test/fixtures/permission/fs-wildcard.js | 57 ++++++++ .../permission/fs-write.js} | 32 ++--- ...est-permission-allow-child-process-cli.js} | 0 ...js => test-permission-allow-worker-cli.js} | 0 ...s => test-permission-child-process-cli.js} | 0 .../test-permission-deny-child-process.js | 70 ---------- .../test-permission-deny-fs-wildcard.js | 131 ------------------ .../test-permission-deny-worker-threads.js | 32 ----- test/parallel/test-permission-deny.js | 97 ------------- test/parallel/test-permission-fs-read.js | 47 +++++++ .../test-permission-fs-relative-path.js | 18 --- ...test-permission-fs-symlink-target-write.js | 57 ++++++++ test/parallel/test-permission-fs-symlink.js | 58 ++++++++ test/parallel/test-permission-fs-wildcard.js | 93 +++++++++++++ .../test-permission-fs-windows-path.js | 60 +++----- test/parallel/test-permission-fs-write.js | 45 ++++++ test/parallel/test-permission-has.js | 23 +++ ... => test-permission-worker-threads-cli.js} | 0 36 files changed, 451 insertions(+), 797 deletions(-) delete mode 100644 benchmark/permission/permission-fs-deny.js rename benchmark/permission/{permission-fs-is-granted.js => permission-fs-read.js} (62%) rename test/{parallel/test-permission-deny-fs-read.js => fixtures/permission/fs-read.js} (64%) rename test/{parallel/test-permission-deny-fs-symlink-target-write.js => fixtures/permission/fs-symlink-target-write.js} (64%) rename test/{parallel/test-permission-deny-fs-symlink.js => fixtures/permission/fs-symlink.js} (71%) create mode 100644 test/fixtures/permission/fs-wildcard.js rename test/{parallel/test-permission-deny-fs-write.js => fixtures/permission/fs-write.js} (86%) rename test/parallel/{test-permission-deny-allow-child-process-cli.js => test-permission-allow-child-process-cli.js} (100%) rename test/parallel/{test-permission-deny-allow-worker-cli.js => test-permission-allow-worker-cli.js} (100%) rename test/parallel/{test-permission-deny-child-process-cli.js => test-permission-child-process-cli.js} (100%) delete mode 100644 test/parallel/test-permission-deny-child-process.js delete mode 100644 test/parallel/test-permission-deny-fs-wildcard.js delete mode 100644 test/parallel/test-permission-deny-worker-threads.js delete mode 100644 test/parallel/test-permission-deny.js create mode 100644 test/parallel/test-permission-fs-read.js create mode 100644 test/parallel/test-permission-fs-symlink-target-write.js create mode 100644 test/parallel/test-permission-fs-symlink.js create mode 100644 test/parallel/test-permission-fs-wildcard.js create mode 100644 test/parallel/test-permission-fs-write.js create mode 100644 test/parallel/test-permission-has.js rename test/parallel/{test-permission-deny-worker-threads-cli.js => test-permission-worker-threads-cli.js} (100%) diff --git a/benchmark/permission/permission-fs-deny.js b/benchmark/permission/permission-fs-deny.js deleted file mode 100644 index 29bbeb27dc7c97..00000000000000 --- a/benchmark/permission/permission-fs-deny.js +++ /dev/null @@ -1,19 +0,0 @@ -'use strict'; -const common = require('../common.js'); - -const configs = { - n: [1e5], - concurrent: [1, 10], -}; - -const options = { flags: ['--experimental-permission'] }; - -const bench = common.createBenchmark(main, configs, options); - -async function main(conf) { - bench.start(); - for (let i = 0; i < conf.n; i++) { - process.permission.deny('fs.read', ['/home/example-file-' + i]); - } - bench.end(conf.n); -} diff --git a/benchmark/permission/permission-fs-is-granted.js b/benchmark/permission/permission-fs-read.js similarity index 62% rename from benchmark/permission/permission-fs-is-granted.js rename to benchmark/permission/permission-fs-read.js index 062ba1944578f4..bd81814e55861a 100644 --- a/benchmark/permission/permission-fs-is-granted.js +++ b/benchmark/permission/permission-fs-read.js @@ -1,6 +1,5 @@ 'use strict'; const common = require('../common.js'); -const fs = require('fs/promises'); const path = require('path'); const configs = { @@ -19,22 +18,11 @@ const options = { const bench = common.createBenchmark(main, configs, options); -const recursivelyDenyFiles = async (dir) => { - const files = await fs.readdir(dir, { withFileTypes: true }); - for (const file of files) { - if (file.isDirectory()) { - await recursivelyDenyFiles(path.join(dir, file.name)); - } else if (file.isFile()) { - process.permission.deny('fs.read', [path.join(dir, file.name)]); - } - } -}; - +// This is a naive benchmark and might not demonstrate real-world use cases. +// New benchmarks will be created once the permission model config is available +// through a config file. async function main(conf) { const benchmarkDir = path.join(__dirname, '../..'); - // Get all the benchmark files and deny access to it - await recursivelyDenyFiles(benchmarkDir); - bench.start(); for (let i = 0; i < conf.n; i++) { diff --git a/doc/api/permissions.md b/doc/api/permissions.md index b31d2934b3aad2..90051f18810e73 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -492,24 +492,7 @@ using the [`--allow-child-process`][] and [`--allow-worker`][] respectively. When enabling the Permission Model through the [`--experimental-permission`][] flag a new property `permission` is added to the `process` object. -This property contains two functions: - -##### `permission.deny(scope [,parameters])` - -API call to deny permissions at runtime ([`permission.deny()`][]) - -```js -process.permission.deny('fs'); // Deny permissions to ALL fs operations - -// Deny permissions to ALL FileSystemWrite operations -process.permission.deny('fs.write'); -// deny FileSystemWrite permissions to the protected-folder -process.permission.deny('fs.write', ['/home/rafaelgss/protected-folder']); -// Deny permissions to ALL FileSystemRead operations -process.permission.deny('fs.read'); -// deny FileSystemRead permissions to the protected-folder -process.permission.deny('fs.read', ['/home/rafaelgss/protected-folder']); -``` +This property contains one function: ##### `permission.has(scope ,parameters)` @@ -519,10 +502,8 @@ API call to check permissions at runtime ([`permission.has()`][]) process.permission.has('fs.write'); // true process.permission.has('fs.write', '/home/rafaelgss/protected-folder'); // true -process.permission.deny('fs.write', '/home/rafaelgss/protected-folder'); - -process.permission.has('fs.write'); // true -process.permission.has('fs.write', '/home/rafaelgss/protected-folder'); // false +process.permission.has('fs.read'); // true +process.permission.has('fs.read', '/home/rafaelgss/protected-folder'); // false ``` #### File System Permissions @@ -560,31 +541,11 @@ There are constraints you need to know before using this system: * Native modules are restricted by default when using the Permission Model. * Relative paths are not supported through the CLI (`--allow-fs-*`). - The runtime API supports relative paths. * The model does not inherit to a child node process. * The model does not inherit to a worker thread. * When creating symlinks the target (first argument) should have read and write access. * Permission changes are not retroactively applied to existing resources. - Consider the following snippet: - ```js - const fs = require('node:fs'); - - // Open a fd - const fd = fs.openSync('./README.md', 'r'); - // Then, deny access to all fs.read operations - process.permission.deny('fs.read'); - // This call will NOT fail and the file will be read - const data = fs.readFileSync(fd); - ``` - -Therefore, when possible, apply the permissions rules before any statement: - -```js -process.permission.deny('fs.read'); -const fd = fs.openSync('./README.md', 'r'); -// Error: Access to this API has been restricted -``` [Security Policy]: https://github.com/nodejs/node/blob/main/SECURITY.md [`--allow-child-process`]: cli.md#--allow-child-process @@ -592,7 +553,6 @@ const fd = fs.openSync('./README.md', 'r'); [`--allow-fs-write`]: cli.md#--allow-fs-write [`--allow-worker`]: cli.md#--allow-worker [`--experimental-permission`]: cli.md#--experimental-permission -[`permission.deny()`]: process.md#processpermissiondenyscope-reference [`permission.has()`]: process.md#processpermissionhasscope-reference [import maps]: https://url.spec.whatwg.org/#relative-url-with-fragment-string [relative-url string]: https://url.spec.whatwg.org/#relative-url-with-fragment-string diff --git a/doc/api/process.md b/doc/api/process.md index a7594aa205b57d..6a82db4d64142a 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -2634,34 +2634,6 @@ This API is available through the [`--experimental-permission`][] flag. for the current process. Additional documentation is available in the [Permission Model][]. -### `process.permission.deny(scope[, reference])` - - - -* `scopes` {string} -* `reference` {Array} -* Returns: {boolean} - -Deny permissions at runtime. - -The available scopes are: - -* `fs` - All File System -* `fs.read` - File System read operations -* `fs.write` - File System write operations - -The reference has a meaning based on the provided scope. For example, -the reference when the scope is File System means files and folders. - -```js -// Deny READ operations to the ./README.md file -process.permission.deny('fs.read', ['./README.md']); -// Deny ALL WRITE operations -process.permission.deny('fs.write'); -``` - ### `process.permission.has(scope[, reference])`