permission: drop process.permission.deny

Author: RafaelGSS

benchmark/permission/permission-fs-deny.js

@@ -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,39 +541,18 @@ 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
 [`--allow-fs-read`]: cli.md#--allow-fs-read
 [`--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

doc/api/permissions.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])`
-
-<!-- YAML
-added: REPLACEME
--->
-
-* `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])`
 
 <!-- YAML

doc/api/process.md

@@ -784,10 +784,10 @@ Environment::Environment(IsolateData* isolate_data,
     if (!options_->allow_fs_read.empty() || !options_->allow_fs_write.empty()) {
       options_->allow_native_addons = false;
       if (!options_->allow_child_process) {
-        permission()->Deny(permission::PermissionScope::kChildProcess, {});
+        permission()->Apply("*", permission::PermissionScope::kChildProcess);
       }
       if (!options_->allow_worker_threads) {
-        permission()->Deny(permission::PermissionScope::kWorkerThreads, {});
+        permission()->Apply("*", permission::PermissionScope::kWorkerThreads);
       }
     }
 

src/env.cc

@@ -10,12 +10,8 @@ namespace permission {
 // Currently, ChildProcess manage a single state
 // Once denied, it's always denied
 void ChildProcessPermission::Apply(const std::string& deny,
-                                   PermissionScope scope) {}
-
-bool ChildProcessPermission::Deny(PermissionScope perm,
-                                  const std::vector<std::string>& params) {
+                                   PermissionScope scope) {
   deny_all_ = true;
-  return true;
 }
 
 bool ChildProcessPermission::is_granted(PermissionScope perm,

src/permission/child_process_permission.cc

@@ -13,8 +13,6 @@ namespace permission {
 class ChildProcessPermission final : public PermissionBase {
  public:
   void Apply(const std::string& deny, PermissionScope scope) override;
-  bool Deny(PermissionScope scope,
-            const std::vector<std::string>& params) override;
   bool is_granted(PermissionScope perm,
                   const std::string_view& param = "") override;
 

src/permission/child_process_permission.h

@@ -48,8 +48,7 @@ void FreeRecursivelyNode(
   delete node;
 }
 
-bool is_tree_granted(node::permission::FSPermission::RadixTree* deny_tree,
-                     node::permission::FSPermission::RadixTree* granted_tree,
+bool is_tree_granted(node::permission::FSPermission::RadixTree* granted_tree,
                      const std::string_view& param) {
 #ifdef _WIN32
   // is UNC file path
@@ -60,11 +59,10 @@ bool is_tree_granted(node::permission::FSPermission::RadixTree* deny_tree,
       starting_pos += 4;  // "UNC\"
     }
     auto normalized = param.substr(starting_pos);
-    return !deny_tree->Lookup(normalized) &&
-           granted_tree->Lookup(normalized, true);
+    return granted_tree->Lookup(normalized, true);
   }
 #endif
-  return !deny_tree->Lookup(param) && granted_tree->Lookup(param, true);
+  return granted_tree->Lookup(param, true);
 }
 
 }  // namespace
@@ -91,40 +89,6 @@ void FSPermission::Apply(const std::string& allow, PermissionScope scope) {
   }
 }
 
-bool FSPermission::Deny(PermissionScope perm,
-                        const std::vector<std::string>& params) {
-  if (perm == PermissionScope::kFileSystem) {
-    deny_all_in_ = true;
-    deny_all_out_ = true;
-    return true;
-  }
-
-  bool deny_all = params.size() == 0;
-  if (perm == PermissionScope::kFileSystemRead) {
-    if (deny_all) deny_all_in_ = true;
-    // when deny_all_in is already true permission.deny should be idempotent
-    if (deny_all_in_) return true;
-    allow_all_in_ = false;
-    for (auto& param : params) {
-      deny_in_fs_.Insert(WildcardIfDir(param));
-    }
-    return true;
-  }
-
-  if (perm == PermissionScope::kFileSystemWrite) {
-    if (deny_all) deny_all_out_ = true;
-    // when deny_all_out is already true permission.deny should be idempotent
-    if (deny_all_out_) return true;
-    allow_all_out_ = false;
-
-    for (auto& param : params) {
-      deny_out_fs_.Insert(WildcardIfDir(param));
-    }
-    return true;
-  }
-  return false;
-}
-
 void FSPermission::GrantAccess(PermissionScope perm, std::string res) {
   const std::string path = WildcardIfDir(res);
   if (perm == PermissionScope::kFileSystemRead) {
@@ -144,11 +108,11 @@ bool FSPermission::is_granted(PermissionScope perm,
     case PermissionScope::kFileSystemRead:
       return !deny_all_in_ &&
              ((param.empty() && allow_all_in_) || allow_all_in_ ||
-              is_tree_granted(&deny_in_fs_, &granted_in_fs_, param));
+              is_tree_granted(&granted_in_fs_, param));
     case PermissionScope::kFileSystemWrite:
       return !deny_all_out_ &&
              ((param.empty() && allow_all_out_) || allow_all_out_ ||
-              is_tree_granted(&deny_out_fs_, &granted_out_fs_, param));
+              is_tree_granted(&granted_out_fs_, param));
     default:
       return false;
   }

src/permission/fs_permission.cc

@@ -17,8 +17,6 @@ namespace permission {
 class FSPermission final : public PermissionBase {
  public:
   void Apply(const std::string& deny, PermissionScope scope) override;
-  bool Deny(PermissionScope scope,
-            const std::vector<std::string>& params) override;
   bool is_granted(PermissionScope perm, const std::string_view& param) override;
 
   // For debugging purposes, use the gist function to print the whole tree
@@ -135,18 +133,9 @@ class FSPermission final : public PermissionBase {
   void GrantAccess(PermissionScope scope, std::string param);
   void RestrictAccess(PermissionScope scope,
                       const std::vector<std::string>& params);
-  // /tmp/* --grant
-  // /tmp/dsadsa/t.js denied in runtime
-  //
-  // /tmp/text.txt -- grant
-  // /tmp/text.txt -- denied in runtime
-  //
   // fs granted on startup
   RadixTree granted_in_fs_;
   RadixTree granted_out_fs_;
-  // fs denied in runtime
-  RadixTree deny_in_fs_;
-  RadixTree deny_out_fs_;
 
   bool deny_all_in_ = true;
   bool deny_all_out_ = true;

src/permission/fs_permission.h

@@ -14,10 +14,8 @@
 
 namespace node {
 
-using v8::Array;
 using v8::Context;
 using v8::FunctionCallbackInfo;
-using v8::Integer;
 using v8::Local;
 using v8::Object;
 using v8::String;
@@ -27,42 +25,6 @@ namespace permission {
 
 namespace {
 
-// permission.deny('fs.read', ['/tmp/'])
-// permission.deny('fs.read')
-static void Deny(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  v8::Isolate* isolate = env->isolate();
-  CHECK(args[0]->IsString());
-  std::string deny_scope = *String::Utf8Value(isolate, args[0]);
-  PermissionScope scope = Permission::StringToPermission(deny_scope);
-  if (scope == PermissionScope::kPermissionsRoot) {
-    return args.GetReturnValue().Set(false);
-  }
-
-  std::vector<std::string> params;
-  if (args.Length() == 1 || args[1]->IsUndefined()) {
-    return args.GetReturnValue().Set(env->permission()->Deny(scope, params));
-  }
-
-  CHECK(args[1]->IsArray());
-  Local<Array> js_params = Local<Array>::Cast(args[1]);
-  Local<Context> context = isolate->GetCurrentContext();
-
-  for (uint32_t i = 0; i < js_params->Length(); ++i) {
-    Local<Value> arg;
-    if (!js_params->Get(context, Integer::New(isolate, i)).ToLocal(&arg)) {
-      return;
-    }
-    String::Utf8Value utf8_arg(isolate, arg);
-    if (*utf8_arg == nullptr) {
-      return;
-    }
-    params.push_back(*utf8_arg);
-  }
-
-  return args.GetReturnValue().Set(env->permission()->Deny(scope, params));
-}
-
 // permission.has('fs.in', '/tmp/')
 // permission.has('fs.in')
 static void Has(const FunctionCallbackInfo<Value>& args) {
@@ -169,27 +131,16 @@ void Permission::Apply(const std::string& allow, PermissionScope scope) {
   }
 }
 
-bool Permission::Deny(PermissionScope scope,
-                      const std::vector<std::string>& params) {
-  auto permission = nodes_.find(scope);
-  if (permission != nodes_.end()) {
-    return permission->second->Deny(scope, params);
-  }
-  return false;
-}
-
 void Initialize(Local<Object> target,
                 Local<Value> unused,
                 Local<Context> context,
                 void* priv) {
-  SetMethod(context, target, "deny", Deny);
   SetMethodNoSideEffect(context, target, "has", Has);
 
   target->SetIntegrityLevel(context, v8::IntegrityLevel::kFrozen).FromJust();
 }
 
 void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
-  registry->Register(Deny);
   registry->Register(Has);
 }
 

src/permission/permission.cc

@@ -47,8 +47,6 @@ class Permission {
 
   // CLI Call
   void Apply(const std::string& deny, PermissionScope scope);
-  // Permission.Deny API
-  bool Deny(PermissionScope scope, const std::vector<std::string>& params);
   void EnablePermissions();
 
  private:

src/permission/permission.h

@@ -37,8 +37,6 @@ enum class PermissionScope {
 class PermissionBase {
  public:
   virtual void Apply(const std::string& deny, PermissionScope scope) = 0;
-  virtual bool Deny(PermissionScope scope,
-                    const std::vector<std::string>& params) = 0;
   virtual bool is_granted(PermissionScope perm,
                           const std::string_view& param = "") = 0;
 };

src/permission/permission_base.h

@@ -9,12 +9,8 @@ namespace permission {
 
 // Currently, PolicyDenyWorker manage a single state
 // Once denied, it's always denied
-void WorkerPermission::Apply(const std::string& deny, PermissionScope scope) {}
-
-bool WorkerPermission::Deny(PermissionScope perm,
-                            const std::vector<std::string>& params) {
+void WorkerPermission::Apply(const std::string& deny, PermissionScope scope) {
   deny_all_ = true;
-  return true;
 }
 
 bool WorkerPermission::is_granted(PermissionScope perm,

src/permission/worker_permission.cc

@@ -13,8 +13,6 @@ namespace permission {
 class WorkerPermission final : public PermissionBase {
  public:
   void Apply(const std::string& deny, PermissionScope scope) override;
-  bool Deny(PermissionScope scope,
-            const std::vector<std::string>& params) override;
   bool is_granted(PermissionScope perm,
                   const std::string_view& param = "") override;