From fcf16079e9d7701c164c3e87725402d7d444b923 Mon Sep 17 00:00:00 2001
From: RafaelGSS <rafael.nunu@hotmail.com>
Date: Tue, 26 Jul 2022 22:12:26 -0300
Subject: [PATCH] test: fix policy-deny tests

---
 doc/api/cli.md                            |  11 ++
 doc/node.1                                |   3 +
 lib/internal/repl/history.js              |  13 +-
 src/node_file.cc                          |  19 ++-
 src/policy/policy.h                       |   2 +-
 test/fixtures/policy/deny/regular-file.md |   3 -
 test/parallel/test-bootstrap-modules.js   |   4 +
 test/parallel/test-cli-policy-deny-fs.js  |  15 +-
 test/parallel/test-policy-deny-fs-in.js   | 171 +++++++++-------------
 test/parallel/test-policy-deny-fs-out.js  | 143 ++++--------------
 test/parallel/test-policy-deny.js         |  22 +--
 11 files changed, 157 insertions(+), 249 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 3db570deb695bc..4c674b311137ed 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -783,6 +783,16 @@ 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.
 
+### `--policy-deny-fs`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+TODO
+
 ### `--policy-integrity=sri`
 
 <!-- YAML
@@ -1650,6 +1660,7 @@ Node.js options that are allowed are:
 * `--openssl-config`
 * `--openssl-legacy-provider`
 * `--pending-deprecation`
+* `--policy-deny-fs`
 * `--policy-integrity`
 * `--preserve-symlinks-main`
 * `--preserve-symlinks`
diff --git a/doc/node.1 b/doc/node.1
index 2003bc8ca79127..1060104ae455c7 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -309,6 +309,9 @@ Among other uses, this can be used to enable FIPS-compliant crypto if Node.js is
 .It Fl -pending-deprecation
 Emit pending deprecation warnings.
 .
+.It Fl -policy-deny-fs
+Instructs Node.js to restrict access to the given resource type
+.
 .It Fl -policy-integrity Ns = Ns Ar sri
 Instructs Node.js to error prior to running any code if the policy does not have the specified integrity. It expects a Subresource Integrity string as a parameter.
 .
diff --git a/lib/internal/repl/history.js b/lib/internal/repl/history.js
index 4f4f7abf0e358d..c199da37f0bda2 100644
--- a/lib/internal/repl/history.js
+++ b/lib/internal/repl/history.js
@@ -40,13 +40,6 @@ function setupHistory(repl, historyPath, ready) {
     return ready(null, repl);
   }
 
-  // TODO: make it more granular (fs.in os.homedir())
-  if (!process.policy.check('fs.in') || !process.policy.check('fs.out')) {
-    _writeToOutput(repl, '\nAccess to FileSystemIn/Out is restricted.\n' +
-      'REPL session history will not be persisted.\n');
-    return ready(null, repl);
-  }
-
   if (!historyPath) {
     try {
       historyPath = path.join(os.homedir(), '.node_repl_history');
@@ -60,6 +53,12 @@ function setupHistory(repl, historyPath, ready) {
     }
   }
 
+  if (!process.policy.check('fs.out', historyPath)) {
+    _writeToOutput(repl, '\nAccess to FileSystemOut is restricted.\n' +
+      'REPL session history will not be persisted.\n');
+    return ready(null, repl);
+  }
+
   let timer = null;
   let writing = false;
   let pending = false;
diff --git a/src/node_file.cc b/src/node_file.cc
index 83e61bf58ea293..66efbb01c692b3 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -1235,8 +1235,13 @@ static void Rename(const FunctionCallbackInfo<Value>& args) {
 
   BufferValue old_path(isolate, args[0]);
   CHECK_NOT_NULL(*old_path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, policy::Permission::kFileSystemOut, *old_path);
+
   BufferValue new_path(isolate, args[1]);
   CHECK_NOT_NULL(*new_path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, policy::Permission::kFileSystemOut, *new_path);
 
   FSReqBase* req_wrap_async = GetReqWrap(args, 2);
   if (req_wrap_async != nullptr) {
@@ -1353,6 +1358,8 @@ static void RMDir(const FunctionCallbackInfo<Value>& args) {
 
   BufferValue path(env->isolate(), args[0]);
   CHECK_NOT_NULL(*path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, policy::Permission::kFileSystemOut, *path);
 
   FSReqBase* req_wrap_async = GetReqWrap(args, 1);  // rmdir(path, req)
   if (req_wrap_async != nullptr) {
@@ -1557,6 +1564,8 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
 
   BufferValue path(env->isolate(), args[0]);
   CHECK_NOT_NULL(*path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, policy::Permission::kFileSystemOut, *path);
 
   CHECK(args[1]->IsInt32());
   const int mode = args[1].As<Int32>()->Value();
@@ -1755,7 +1764,7 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
         env, policy::Permission::kFileSystemIn, *path);
     THROW_IF_INSUFFICIENT_PERMISSIONS(
         env, policy::Permission::kFileSystemOut, *path);
-  } else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) {
+  } else if ((flags & ~(O_RDONLY | O_SYNC)) == 0) {
     THROW_IF_INSUFFICIENT_PERMISSIONS(
         env, policy::Permission::kFileSystemIn, *path);
   } else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) {
@@ -1806,7 +1815,7 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
         env, policy::Permission::kFileSystemIn, *path);
     THROW_IF_INSUFFICIENT_PERMISSIONS(
         env, policy::Permission::kFileSystemOut, *path);
-  } else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) {
+  } else if ((flags & ~(O_RDONLY | O_SYNC)) == 0) {
     THROW_IF_INSUFFICIENT_PERMISSIONS(
         env, policy::Permission::kFileSystemIn, *path);
   } else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) {
@@ -1843,9 +1852,13 @@ static void CopyFile(const FunctionCallbackInfo<Value>& args) {
 
   BufferValue src(isolate, args[0]);
   CHECK_NOT_NULL(*src);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, policy::Permission::kFileSystemIn, *src);
 
   BufferValue dest(isolate, args[1]);
   CHECK_NOT_NULL(*dest);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, policy::Permission::kFileSystemOut, *dest);
 
   CHECK(args[2]->IsInt32());
   const int flags = args[2].As<Int32>()->Value();
@@ -2334,6 +2347,8 @@ static void UTimes(const FunctionCallbackInfo<Value>& args) {
 
   BufferValue path(env->isolate(), args[0]);
   CHECK_NOT_NULL(*path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, policy::Permission::kFileSystemOut, *path);
 
   CHECK(args[1]->IsNumber());
   const double atime = args[1].As<Number>()->Value();
diff --git a/src/policy/policy.h b/src/policy/policy.h
index 27b663940f3107..c02bc3eaa76aa8 100644
--- a/src/policy/policy.h
+++ b/src/policy/policy.h
@@ -19,7 +19,7 @@ namespace policy {
 
 #define THROW_IF_INSUFFICIENT_PERMISSIONS(env, perm_, resource_, ...)          \
   if (!node::policy::root_policy.is_granted(perm_, resource_)) {               \
-    return node::policy::Policy::ThrowAccessDenied((env), perm_);                     \
+    return node::policy::Policy::ThrowAccessDenied((env), perm_);              \
   }
 
 class Policy {
diff --git a/test/fixtures/policy/deny/regular-file.md b/test/fixtures/policy/deny/regular-file.md
index eb6054485b6fb4..e69de29bb2d1d6 100644
--- a/test/fixtures/policy/deny/regular-file.md
+++ b/test/fixtures/policy/deny/regular-file.md
@@ -1,3 +0,0 @@
-# Regular File
-
-Example of a regular file to be used in the PolicyDenyFs module
diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js
index d5bf6e133ebc99..c5b0b60212651d 100644
--- a/test/parallel/test-bootstrap-modules.js
+++ b/test/parallel/test-bootstrap-modules.js
@@ -27,6 +27,7 @@ const expectedModules = new Set([
   'Internal Binding options',
   'Internal Binding performance',
   'Internal Binding pipe_wrap',
+  'Internal Binding policy',
   'Internal Binding process_methods',
   'Internal Binding report',
   'Internal Binding serdes',
@@ -100,10 +101,13 @@ const expectedModules = new Set([
   'NativeModule internal/perf/timerify',
   'NativeModule internal/perf/usertiming',
   'NativeModule internal/perf/utils',
+  'NativeModule internal/policy/manifest',
+  'NativeModule internal/policy/sri',
   'NativeModule internal/priority_queue',
   'NativeModule internal/process/esm_loader',
   'NativeModule internal/process/execution',
   'NativeModule internal/process/per_thread',
+  'NativeModule internal/process/policy',
   'NativeModule internal/process/promises',
   'NativeModule internal/process/report',
   'NativeModule internal/process/signal',
diff --git a/test/parallel/test-cli-policy-deny-fs.js b/test/parallel/test-cli-policy-deny-fs.js
index 2d3fc13ab45993..f8a1b54fca5315 100644
--- a/test/parallel/test-cli-policy-deny-fs.js
+++ b/test/parallel/test-cli-policy-deny-fs.js
@@ -4,12 +4,9 @@ const common = require('../common');
 if (!common.hasCrypto)
   common.skip('missing crypto');
 
-const fixtures = require('../common/fixtures');
-
 const { spawnSync } = require('child_process');
 const assert = require('assert');
-
-const dep = fixtures.path('policy', 'deny', 'check.js');
+const fs = require('fs');
 
 {
   const { status, stdout } = spawnSync(
@@ -18,7 +15,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
       '--policy-deny-fs', 'fs', '-e',
       `console.log(process.policy.check("fs"));
        console.log(process.policy.check("fs.in"));
-       console.log(process.policy.check("fs.out"));`
+       console.log(process.policy.check("fs.out"));`,
     ]
   );
 
@@ -37,7 +34,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
       '--policy-deny-fs', 'in', '-e',
       `console.log(process.policy.check("fs"));
        console.log(process.policy.check("fs.in"));
-       console.log(process.policy.check("fs.out"));`
+       console.log(process.policy.check("fs.out"));`,
     ]
   );
 
@@ -55,7 +52,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
       '--policy-deny-fs', 'out', '-e',
       `console.log(process.policy.check("fs"));
        console.log(process.policy.check("fs.in"));
-       console.log(process.policy.check("fs.out"));`
+       console.log(process.policy.check("fs.out"));`,
     ]
   );
 
@@ -73,7 +70,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
       '--policy-deny-fs', 'out,in', '-e',
       `console.log(process.policy.check("fs"));
        console.log(process.policy.check("fs.in"));
-       console.log(process.policy.check("fs.out"));`
+       console.log(process.policy.check("fs.out"));`,
     ]
   );
 
@@ -112,5 +109,5 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
     stderr.toString().includes('Access to this API has been restricted'),
     stderr);
   assert.strictEqual(status, 1);
-  assert.ok(fs.existsSync('policy-deny-example.md'));
+  assert.ok(!fs.existsSync('policy-deny-example.md'));
 }
diff --git a/test/parallel/test-policy-deny-fs-in.js b/test/parallel/test-policy-deny-fs-in.js
index 39ebee0c03136f..00835410b7c737 100644
--- a/test/parallel/test-policy-deny-fs-in.js
+++ b/test/parallel/test-policy-deny-fs-in.js
@@ -6,33 +6,12 @@ if (!common.hasCrypto)
   common.skip('missing crypto');
 
 const assert = require('assert');
-const fs = require('fs')
+const fs = require('fs');
 
 const blockedFile = '.gitignore';
 const blockedFolder = '/tmp/';
 const regularFile = __filename;
 
-// fs.readFileSync
-{
-  assert.throws(() => {
-    fs.readFileSync(blockedFile);
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemIn',
-  }));
-
-  assert.throws(() => {
-    fs.readFileSync(blockedFolder + 'anyfile');
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemIn',
-  }));
-
-  assert.doesNotThrow(() => {
-    fs.readFileSync(regularFile);
-  });
-}
-
 // fs.readFile
 {
   assert.throws(() => {
@@ -49,9 +28,8 @@ const regularFile = __filename;
     permission: 'FileSystemIn',
   }));
 
-  assert.doesNotThrow(() => {
-    fs.readFile(regularFile, () => {});
-  });
+  // doesNotThrow
+  fs.readFile(regularFile, () => {});
 }
 
 // fs.createReadStream
@@ -59,8 +37,8 @@ const regularFile = __filename;
   assert.rejects(() => {
     return new Promise((_resolve, reject) => {
       const stream = fs.createReadStream(blockedFile);
-      stream.on('error', reject)
-    })
+      stream.on('error', reject);
+    });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
@@ -69,119 +47,102 @@ const regularFile = __filename;
   assert.rejects(() => {
     return new Promise((_resolve, reject) => {
       const stream = fs.createReadStream(blockedFile);
-      stream.on('error', reject)
-    })
+      stream.on('error', reject);
+    });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
-
-  assert.doesNotReject(() => {
-    return new Promise((_resolve, reject) => {
-      const stream = fs.createReadStream(regularFile);
-      stream.on('error', reject)
-    })
-  });
 }
 
-// fs.statSync
+// fs.stat
 {
   assert.throws(() => {
-    fs.statSync(blockedFile);
+    fs.stat(blockedFile, () => {});
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
 
   assert.throws(() => {
-    fs.statSync(blockedFolder + 'anyfile');
+    fs.stat(blockedFolder + 'anyfile', () => {});
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
 
-  assert.doesNotThrow(() => {
-    fs.statSync(regularFile);
+  // doesNotThrow
+  fs.stat(regularFile, (err) => {
+    assert.ifError(err);
   });
 }
 
-// fs.stat
+// fs.access
 {
   assert.throws(() => {
-    fs.stat(blockedFile, () => {});
+    fs.access(blockedFile, fs.constants.R_OK, () => {});
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
 
   assert.throws(() => {
-    fs.stat(blockedFolder + 'anyfile', () => {});
+    fs.access(blockedFolder + 'anyfile', fs.constants.R_OK, () => {});
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
 
-  assert.doesNotThrow(() => {
-    fs.stat(regularFile, () => {});
+  // doesNotThrow
+  fs.access(regularFile, fs.constants.R_OK, (err) => {
+    assert.ifError(err);
   });
 }
 
-// fs.access
+// fs.chownSync (should not bypass)
 {
   assert.throws(() => {
-    fs.access(blockedFile, fs.constants.R_OK, () => {});
+    // This operation will work fine
+    fs.chownSync(blockedFile, process.getuid(), process.getgid());
+    fs.readFileSync(blockedFile);
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
+}
 
+// fs.copyFile
+{
   assert.throws(() => {
-    fs.access(blockedFolder + 'anyfile', fs.constants.R_OK, () => {});
+    fs.copyFile(blockedFile, blockedFolder + 'any-other-file', () => {});
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
 
-  assert.doesNotThrow(() => {
-    fs.access(regularFile, fs.constants.R_OK, () => {});
-  });
-}
-
-// fs.chownSync (should not bypass)
-{
   assert.throws(() => {
-    // this operation will work fine
-    fs.chownSync(blockedFile, process.getuid(), process.getgid());
-    fs.readFileSync(blockedFile)
+    fs.copyFile(blockedFile, `${__dirname}/any-other-file`, () => {});
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
 }
 
-// // TODO(rafaelgss): mention possible workarounds (spawn('cp blockedFile regularFile'))
-// // copyFile (handle security concerns)
-// // cp (handle security concerns)
-
-// fs.openSync
+// fs.cp
 {
   assert.throws(() => {
-    fs.openSync(blockedFile, 'r');
+    fs.cpSync(blockedFile, blockedFolder + 'any-other-file');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
 
   assert.throws(() => {
-    fs.openSync(blockedFolder + 'anyfile', 'r');
+    fs.cpSync(blockedFile, `${__dirname}/any-other-file`);
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
-
-  assert.doesNotThrow(() => {
-    fs.openSync(regularFile, 'r');
-  });
 }
 
 // fs.open
@@ -200,51 +161,55 @@ const regularFile = __filename;
     permission: 'FileSystemIn',
   }));
 
-  assert.doesNotThrow(() => {
-    fs.open(regularFile, 'r', () => {});
+  // doesNotThrow
+  fs.open(regularFile, 'r', (err) => {
+    assert.ifError(err);
   });
 }
 
-// fs.opendir (TODO)
+// fs.opendir
 {
   assert.throws(() => {
     fs.opendir(blockedFolder, (err) => {
-      if (err) throw err;
+      assert.ifError(err);
     });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemIn',
   }));
 
-  assert.doesNotThrow(() => {
-    fs.opendir(__dirname, () => {});
+  // doesNotThrow
+  fs.opendir(__dirname, (err) => {
+    assert.ifError(err);
+  });
+}
+
+// fs.readdir
+{
+  assert.throws(() => {
+    fs.readdir(blockedFolder, () => {});
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'FileSystemIn',
+  }));
+
+  // doesNotThrow
+  fs.readdir(__dirname, (err) => {
+    assert.ifError(err);
   });
 }
 
-// // fs.readdir
-// {
-//   // assert.throws(() => {
-//   //   fs.readdir(blockedFolder, () => {});
-//   // }, common.expectsError({
-//   //   code: 'ERR_ACCESS_DENIED',
-//   //   permission: 'FileSystemIn',
-//   // }));
-
-//   assert.doesNotThrow(() => {
-//     fs.readdir(__dirname, () => {});
-//   });
-// }
-
-// // fs.watch (TODO)
-// {
-//   assert.throws(() => {
-//     fs.watch(blockedFile, () => {});
-//   }, common.expectsError({
-//     code: 'ERR_ACCESS_DENIED',
-//     permission: 'FileSystemIn',
-//   }));
-
-//   assert.doesNotThrow(() => {
-//     fs.readdir(__dirname, () => {});
-//   });
-// }
+// fs.watch
+{
+  assert.throws(() => {
+    fs.watch(blockedFile, () => {});
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'FileSystemIn',
+  }));
+
+  // doesNotThrow
+  fs.readdir(__dirname, (err) => {
+    assert.ifError(err);
+  });
+}
diff --git a/test/parallel/test-policy-deny-fs-out.js b/test/parallel/test-policy-deny-fs-out.js
index 72c1a86ee3cf06..08de3c2c7377bc 100644
--- a/test/parallel/test-policy-deny-fs-out.js
+++ b/test/parallel/test-policy-deny-fs-out.js
@@ -19,23 +19,6 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
   assert.ok(process.policy.deny('fs.out', blockedFile));
 }
 
-// fs.writeFileSync
-{
-  assert.throws(() => {
-    fs.writeFileSync(blockedFile, 'example');
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemOut',
-  }));
-
-  assert.throws(() => {
-    fs.writeFileSync(blockedFolder + 'anyfile', 'example');
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemOut',
-  }));
-}
-
 // fs.writeFile
 {
   assert.throws(() => {
@@ -46,24 +29,7 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
   }));
 
   assert.throws(() => {
-    fs.writeFile(blockedFolder + 'anyfile', 'example', () => {});
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemOut',
-  }));
-}
-
-// fs.writeSync
-{
-  assert.throws(() => {
-    fs.writeSync(fs.openSync(blockedFile, 'w'), 'example')
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemOut',
-  }));
-
-  assert.throws(() => {
-    fs.writeSync(fs.openSync(blockedFolder + 'anyfile', 'w'), 'example');
+    fs.writeFile(blockedFolder + '/anyfile', 'example', () => {});
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemOut',
@@ -76,7 +42,7 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
     return new Promise((_resolve, reject) => {
       const stream = fs.createWriteStream(blockedFile);
       stream.on('error', reject);
-    })
+    });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemOut',
@@ -84,78 +50,26 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
 
   assert.rejects(() => {
     return new Promise((_resolve, reject) => {
-      const stream = fs.createReadStream(blockedFolder + 'example');
+      const stream = fs.createWriteStream(blockedFolder + '/example');
       stream.on('error', reject);
-    })
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemOut',
-  }));
-}
-
-// fs.utimesSync
-{
-  assert.throws(() => {
-    fs.utimesSync(blockedFile, new Date(), new Date());
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemOut',
-  }));
-
-  assert.throws(() => {
-    fs.utimesSync(blockedFile + 'anyfile', new Date(), new Date());
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemOut',
-  }));
-}
-
-// fs.copyFileSync
-{
-  assert.throws(() => {
-    fs.copyFileSync(blockedFile, blockedFolder + 'any-other-file');
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemOut',
-  }));
-
-  // TODO(rafaelgss): should it throw when copying (reading) from a
-  // blockedFile (out) to a regular folder?
-  assert.throws(() => {
-    fs.copyFileSync(blockedFile, regularFolder + 'any-other-file');
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemOut',
-  }));
-
-  assert.throws(() => {
-    fs.copyFileSync(regularFile, blockedFolder + 'any-file');
+    });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemOut',
   }));
 }
 
-// fs.cpSync
+// fs.utimes
 {
   assert.throws(() => {
-    fs.cpSync(blockedFile, blockedFolder + 'any-other-file');
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemOut',
-  }));
-
-  // TODO(rafaelgss): should it throw when copying (reading) from a
-  // blockedFile (out) to a regular folder?
-  assert.throws(() => {
-    fs.cpSync(blockedFile, regularFolder + 'any-other-file');
+    fs.utimes(blockedFile, new Date(), new Date(), () => {});
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemOut',
   }));
 
   assert.throws(() => {
-    fs.cpSync(regularFile, blockedFolder + 'any-file');
+    fs.utimes(blockedFile + 'anyfile', new Date(), new Date(), () => {});
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemOut',
@@ -165,8 +79,8 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
 // fs.mkdir
 {
   assert.throws(() => {
-    fs.mkdir(blockedFolder + 'any-folder', (err) => {
-      if (err) throw err;
+    fs.mkdir(blockedFolder + '/any-folder', (err) => {
+      assert.ifError(err);
     });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
@@ -178,7 +92,7 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
 {
   assert.throws(() => {
     fs.rename(blockedFile, blockedFile + 'renamed', (err) => {
-      if (err) throw err;
+      assert.ifError(err);
     });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
@@ -186,8 +100,8 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
   }));
 
   assert.throws(() => {
-    fs.rename(blockedFile, regularFolder + 'renamed', (err) => {
-      if (err) throw err;
+    fs.rename(blockedFile, regularFolder + '/renamed', (err) => {
+      assert.ifError(err);
     });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
@@ -195,8 +109,8 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
   }));
 
   assert.throws(() => {
-    fs.rename(regularFile, blockedFolder + 'renamed', (err) => {
-      if (err) throw err;
+    fs.rename(regularFile, blockedFolder + '/renamed', (err) => {
+      assert.ifError(err);
     });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
@@ -204,44 +118,43 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
   }));
 }
 
-// fs.rmdir
+// fs.copyFile
 {
   assert.throws(() => {
-    fs.rmdir(blockedFolder, (err) => {
-      if (err) throw err;
-    });
+    fs.copyFileSync(regularFile, blockedFolder + '/any-file');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemOut',
   }));
+}
 
-  // TODO(rafaelgss): the user should be capable to rmdir of a non-protected
-  // folder but that contains a protected file?
-  // regularFolder contains a protected file
+// fs.cp
+{
   assert.throws(() => {
-    fs.rmdir(regularFolder, (err) => {
-      if (err) throw err;
-    });
+    fs.cpSync(regularFile, blockedFolder + '/any-file');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemOut',
   }));
 }
 
+// fs.rmdir
 {
   assert.throws(() => {
-    fs.rm(blockedFolder, (err) => {
-      if (err) throw err;
+    fs.rmdir(blockedFolder, (err) => {
+      assert.ifError(err);
     });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemOut',
   }));
 
-  // regularFolder contains a protected file
+  // The user shouldn't be capable to rmdir of a non-protected folder
+  // but that contains a protected file.
+  // The regularFolder contains a protected file
   assert.throws(() => {
-    fs.rm(regularFolder, (err) => {
-      if (err) throw err;
+    fs.rmdir(regularFolder, (err) => {
+      assert.ifError(err);
     });
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
diff --git a/test/parallel/test-policy-deny.js b/test/parallel/test-policy-deny.js
index 8df2dee1f62d82..c48398f62a0c37 100644
--- a/test/parallel/test-policy-deny.js
+++ b/test/parallel/test-policy-deny.js
@@ -1,3 +1,5 @@
+'use strict';
+
 const common = require('../common');
 const fs = require('fs');
 const fsPromises = require('node:fs/promises');
@@ -9,7 +11,7 @@ const protectedFolder = fixtures.path('policy', 'deny');
 const protectedFile = fixtures.path('policy', 'deny', 'protected-file.md');
 const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
 
-// assert check and deny exists
+// Assert check and deny exists
 {
   assert.ok(typeof process.policy.check === 'function');
   assert.ok(typeof process.policy.deny === 'function');
@@ -26,9 +28,8 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
   assert.ok(process.policy.check('fs.out', protectedFolder));
   assert.ok(process.policy.check('fs.out', regularFile));
 
-  assert.doesNotReject(() => {
-    return fsPromises.readFile(protectedFile);
-  });
+  // doesNotThrow
+  fs.readFileSync(protectedFile);
 }
 
 // Deny access to fs.in
@@ -50,9 +51,8 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
     permission: 'FileSystemIn',
   }));
 
-  assert.doesNotThrow(() => {
-    fs.open(regularFile, () => {});
-  });
+  // doesNotThrow
+  fs.openSync(regularFile, 'w');
 }
 
 // Deny access to fs.out
@@ -69,9 +69,13 @@ const regularFile = fixtures.path('policy', 'deny', 'regular-file.md');
 
   assert.rejects(() => {
     return fsPromises
-      .writeFile(protectedFolder + '/new-file', 'data');
-   }, common.expectsError({
+     .writeFile(protectedFolder + '/new-file', 'data');
+  }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemOut',
   }));
+
+  assert.throws(() => {
+    fs.openSync(regularFile, 'w');
+  });
 }