src,process: add path delimiter flag to permission

--permission-fs-path-delimiter flag added to permission model.
If not provided default value will be comma.

doc/api/cli.md

@@ -554,7 +554,7 @@ Enable the Permission Model for current process. When enabled, the
 following permissions are restricted:
 
 * File System - manageable through
-  [`--allow-fs-read`][], [`--allow-fs-write`][] flags
+  [`--allow-fs-read`][], [`--allow-fs-write`][] and [`--permission-fs-path-delimiter`][] flags
 * Child Process - manageable through [`--allow-child-process`][] flag
 * Worker Threads - manageable through [`--allow-worker`][] flag
 
@@ -1116,6 +1116,27 @@ unless either the `--pending-deprecation` command-line flag, or the
 are used to provide a kind of selective "early warning" mechanism that
 developers may leverage to detect deprecated API usage.
 
+### `--permission-fs-path-delimiter`
+
+<!-- YAML
+added: v20.0.0
+-->
+
+> Stability: 1 - Experimental
+
+This flag configures file system path delimiter for permissions using
+the [Permission Model][].
+
+Examples can be found in the [File System Permissions][] documentation.
+
+Especial characters in bash as `;` must be escaped or quoted:
+
+```bash
+node --experimental-permission --permission-fs-path-delimiter=\; \
+--allow-fs-read=/path/to/index.js index.js
+```
+
+
 ### `--policy-integrity=sri`
 
 <!-- YAML
@@ -2183,6 +2204,7 @@ Node.js options that are allowed are:
 * `--openssl-legacy-provider`
 * `--openssl-shared-config`
 * `--pending-deprecation`
+* `--permission-fs-path-delimiter`
 * `--policy-integrity`
 * `--preserve-symlinks-main`
 * `--preserve-symlinks`

doc/api/permissions.md

@@ -540,6 +540,24 @@ Wildcards are supported too:
 * `--allow-fs-read=/home/test*` will allow read access to everything
   that matches the wildcard. e.g: `/home/test/file1` or `/home/test2`
 
+##### Accessing files with comma in path
+
+To access files with comma in path you can change the path delimiter using the
+`--permission-fs-path-delimiter` flag to set a value not used in any of the
+paths you want to access.
+
+```console
+$ node --experimental-permission --allow-fs-read="/with,commas_/home" \
+--permission-fs-path-delimiter=_ index.js
+```
+
+Note when using bash special characters like `;` escape or quoting is required.
+
+```console
+$ node --experimental-permission --allow-fs-read="/home/with,commas;/home" \
+--permission-fs-path-delimiter=";" index.js
+```
+
 #### Limitations and known issues
 
 There are constraints you need to know before using this system:

src/env.cc

@@ -867,12 +867,14 @@ Environment::Environment(IsolateData* isolate_data,
 
     if (!options_->allow_fs_read.empty()) {
       permission()->Apply(options_->allow_fs_read,
-                          permission::PermissionScope::kFileSystemRead);
+                          permission::PermissionScope::kFileSystemRead,
+                          {{"delimiter", options_->permission_fs_path_delimiter}});
     }
 
     if (!options_->allow_fs_write.empty()) {
       permission()->Apply(options_->allow_fs_write,
-                          permission::PermissionScope::kFileSystemWrite);
+                          permission::PermissionScope::kFileSystemWrite,
+                          {{"delimiter", options_->permission_fs_path_delimiter}});
     }
   }
 }

src/node_options.cc

@@ -422,6 +422,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "allow permissions to read the filesystem",
             &EnvironmentOptions::allow_fs_read,
             kAllowedInEnvvar);
+  AddOption("--permission-fs-path-delimiter",
+            "set the delimiter for the permissions path",
+            &EnvironmentOptions::permission_fs_path_delimiter,
+            kAllowedInEnvvar);
   AddOption("--allow-fs-write",
             "allow permissions to write in the filesystem",
             &EnvironmentOptions::allow_fs_write,

src/node_options.h

@@ -123,6 +123,7 @@ class EnvironmentOptions : public Options {
   bool experimental_permission = false;
   std::string allow_fs_read;
   std::string allow_fs_write;
+  std::string permission_fs_path_delimiter = ",";
   bool allow_child_process = false;
   bool allow_worker_threads = false;
   bool experimental_repl_await = true;

src/permission/child_process_permission.cc

@@ -10,7 +10,8 @@ namespace permission {
 // Currently, ChildProcess manage a single state
 // Once denied, it's always denied
 void ChildProcessPermission::Apply(const std::string& allow,
-                                   PermissionScope scope) {
+                                   PermissionScope scope,
+                                   const std::unordered_map<std::string, std::string>& options) {
   deny_all_ = true;
 }
 

src/permission/child_process_permission.h

@@ -12,7 +12,9 @@ namespace permission {
 
 class ChildProcessPermission final : public PermissionBase {
  public:
-  void Apply(const std::string& allow, PermissionScope scope) override;
+  void Apply(const std::string& allow,
+             PermissionScope scope,
+             const std::unordered_map<std::string, std::string>& options = {}) override;
   bool is_granted(PermissionScope perm,
                   const std::string_view& param = "") override;
 

src/permission/fs_permission.cc

@@ -115,9 +115,10 @@ void PrintTree(const FSPermission::RadixTree::Node* node, size_t spaces = 0) {
 
 // allow = '*'
 // allow = '/tmp/,/home/example.js'
-void FSPermission::Apply(const std::string& allow, PermissionScope scope) {
+void FSPermission::Apply(const std::string& allow, PermissionScope scope, const std::unordered_map<std::string, std::string>& options) {
   using std::string_view_literals::operator""sv;
-  for (const std::string_view res : SplitString(allow, ","sv)) {
+  std::string delimiter = options.find("delimiter") != options.end() ? options.at("delimiter") : ",";
+  for (const std::string_view res : SplitString(allow, delimiter)) {
     if (res == "*"sv) {
       if (scope == PermissionScope::kFileSystemRead) {
         deny_all_in_ = false;

src/permission/fs_permission.h

@@ -15,7 +15,7 @@ namespace permission {
 
 class FSPermission final : public PermissionBase {
  public:
-  void Apply(const std::string& allow, PermissionScope scope) override;
+  void Apply(const std::string& allow, PermissionScope scope, const std::unordered_map<std::string, std::string>& options = {}) override;
   bool is_granted(PermissionScope perm, const std::string_view& param) override;
 
   struct RadixTree {

src/permission/inspector_permission.cc

@@ -9,7 +9,8 @@ namespace permission {
 // Currently, Inspector manage a single state
 // Once denied, it's always denied
 void InspectorPermission::Apply(const std::string& allow,
-                                PermissionScope scope) {
+                                PermissionScope scope,
+                                const std::unordered_map<std::string, std::string>& options) {
   deny_all_ = true;
 }
 

src/permission/inspector_permission.h

@@ -12,7 +12,9 @@ namespace permission {
 
 class InspectorPermission final : public PermissionBase {
  public:
-  void Apply(const std::string& allow, PermissionScope scope) override;
+  void Apply(const std::string& allow,
+             PermissionScope scope,
+             const std::unordered_map<std::string, std::string>& options = {}) override;
   bool is_granted(PermissionScope perm,
                   const std::string_view& param = "") override;
 

src/permission/permission.cc

@@ -130,10 +130,12 @@ void Permission::EnablePermissions() {
   }
 }
 
-void Permission::Apply(const std::string& allow, PermissionScope scope) {
+void Permission::Apply(const std::string& allow,
+                       PermissionScope scope,
+                       const std::unordered_map<std::string, std::string>& options) {
   auto permission = nodes_.find(scope);
   if (permission != nodes_.end()) {
-    permission->second->Apply(allow, scope);
+    permission->second->Apply(allow, scope, options);
   }
 }
 

src/permission/permission.h

@@ -49,7 +49,7 @@ class Permission {
                                 const std::string_view& res);
 
   // CLI Call
-  void Apply(const std::string& allow, PermissionScope scope);
+  void Apply(const std::string& allow, PermissionScope scope, const std::unordered_map<std::string, std::string>& options = {});
   void EnablePermissions();
 
  private:

src/permission/permission_base.h

@@ -39,7 +39,7 @@ enum class PermissionScope {
 
 class PermissionBase {
  public:
-  virtual void Apply(const std::string& allow, PermissionScope scope) = 0;
+  virtual void Apply(const std::string& allow, PermissionScope scope, const std::unordered_map<std::string, std::string>& options = {}) = 0;
   virtual bool is_granted(PermissionScope perm,
                           const std::string_view& param = "") = 0;
 };

src/permission/worker_permission.cc

@@ -9,7 +9,9 @@ namespace permission {
 
 // Currently, PolicyDenyWorker manage a single state
 // Once denied, it's always denied
-void WorkerPermission::Apply(const std::string& allow, PermissionScope scope) {
+void WorkerPermission::Apply(const std::string& allow,
+                             PermissionScope scope,
+                             const std::unordered_map<std::string, std::string>& options) {
   deny_all_ = true;
 }
 

src/permission/worker_permission.h

@@ -12,7 +12,9 @@ namespace permission {
 
 class WorkerPermission final : public PermissionBase {
  public:
-  void Apply(const std::string& allow, PermissionScope scope) override;
+  void Apply(const std::string& allow,
+             PermissionScope scope,
+             const std::unordered_map<std::string, std::string>& options = {}) override;
   bool is_granted(PermissionScope perm,
                   const std::string_view& param = "") override;
 

test/parallel/test-cli-permission-deny-fs.js

@@ -47,6 +47,54 @@ const path = require('path');
   assert.strictEqual(status, 0);
 }
 
+{
+  const tmpPath = path.resolve('/tmp/');
+  const otherPath = path.resolve('/other-path/');
+  const { status, stdout } = spawnSync(
+    process.execPath,
+    [
+      '--experimental-permission',
+      '--allow-fs-write', `${tmpPath},${otherPath}`, '-e',
+      `console.log(process.permission.has("fs"));
+      console.log(process.permission.has("fs.read"));
+      console.log(process.permission.has("fs.write"));
+      console.log(process.permission.has("fs.write", "/tmp/"));
+      console.log(process.permission.has("fs.write", "/other-path/"));`,
+    ]
+  );
+  const [fs, fsIn, fsOut, fsOutAllowed1, fsOutAllowed2] = stdout.toString().split('\n');
+  assert.strictEqual(fs, 'false');
+  assert.strictEqual(fsIn, 'false');
+  assert.strictEqual(fsOut, 'false');
+  assert.strictEqual(fsOutAllowed1, 'true');
+  assert.strictEqual(fsOutAllowed2, 'true');
+  assert.strictEqual(status, 0);
+}
+
+{
+  const tmpPath = path.resolve('/tmp/');
+  const pathWithComma = path.resolve('/other,path/');
+  const { status, stdout } = spawnSync(
+    process.execPath,
+    [
+      '--experimental-permission',
+      '--allow-fs-write', `${tmpPath};${pathWithComma}`, '--permission-fs-path-delimiter=;', '-e',
+      `console.log(process.permission.has("fs"));
+      console.log(process.permission.has("fs.read"));
+      console.log(process.permission.has("fs.write"));
+      console.log(process.permission.has("fs.write", "/tmp/"));
+      console.log(process.permission.has("fs.write", "/other,path/"));`,
+    ]
+  );
+  const [fs, fsIn, fsOut, fsOutAllowed1, fsOutAllowed2] = stdout.toString().split('\n');
+  assert.strictEqual(fs, 'false');
+  assert.strictEqual(fsIn, 'false');
+  assert.strictEqual(fsOut, 'false');
+  assert.strictEqual(fsOutAllowed1, 'true');
+  assert.strictEqual(fsOutAllowed2, 'true');
+  assert.strictEqual(status, 0);
+}
+
 {
   const { status, stdout } = spawnSync(
     process.execPath,