permission: reject empty reference in drop()

doc/api/permissions.md

@@ -118,6 +118,8 @@ API call to drop permissions at runtime. This operation is **irreversible**.
 
 When called without a reference, the entire scope is dropped. When called
 with a reference, only the permission for that specific resource is revoked.
+The reference must not be an empty string or an empty `Uint8Array`; otherwise
+an `ERR_INVALID_ARG_VALUE` error is thrown.
 Dropping a permission only affects future access checks. It does not close or
 revoke access to resources that are already open, such as file descriptors,
 network sockets, child processes, or worker threads. Applications are

doc/api/process.md

@@ -3231,7 +3231,8 @@ permissions.
 When a reference is provided, only the permission for that specific resource
 is dropped. For example, `process.permission.drop('fs.read', '/etc/myapp')`
 will revoke read access to that directory while keeping other read
-permissions intact.
+permissions intact. The `reference` must not be an empty string or an empty
+`Uint8Array`; otherwise an `ERR_INVALID_ARG_VALUE` error is thrown.
 
 **Important:** You can only drop the exact resource that was explicitly
 granted. The reference passed to `drop()` must match the original grant:

lib/internal/process/permission.js

@@ -10,6 +10,11 @@ const permission = internalBinding('permission');
 const { validateString } = require('internal/validators');
 const { isUint8Array } = require('internal/util/types');
 const { isURL, fileURLToPath } = require('internal/url');
+const {
+  codes: {
+    ERR_INVALID_ARG_VALUE,
+  },
+} = require('internal/errors');
 
 let _permission;
 let _audit;
@@ -63,7 +68,12 @@ module.exports = ObjectFreeze({
   },
   drop(scope, reference) {
     validateString(scope, 'scope');
-    permission.drop(scope, normalizeReference(scope, reference, 'reference'));
+    reference = normalizeReference(scope, reference, 'reference');
+    // An empty reference would otherwise drop the whole scope.
+    if (reference != null && reference.length === 0) {
+      throw new ERR_INVALID_ARG_VALUE('reference', reference, 'must not be empty');
+    }
+    permission.drop(scope, reference);
   },
   availableFlags() {
     if (_ffi === undefined) {

src/permission/permission.cc

@@ -116,15 +116,18 @@ static void Drop(const FunctionCallbackInfo<Value>& args) {
         "owns the process state");
   }
 
-  if (args.Length() > 1 && !args[1]->IsUndefined()) {
+  if (args.Length() > 1 && !args[1]->IsNullOrUndefined()) {
     // BufferValue copies raw bytes out of a Buffer/TypedArray as-is instead
     // of forcing a (potentially lossy) UTF-8 string conversion, since paths
     // are not guaranteed to be valid UTF-8.
     BufferValue resource(env->isolate(), args[1]);
-    if (resource.length() > 0) {
-      env->permission()->Drop(env, scope, resource.ToStringView());
-      return;
+    if (resource.length() == 0) {
+      // An empty reference must not fall through to dropping the whole scope.
+      return THROW_ERR_INVALID_ARG_VALUE(
+          env, "The \"reference\" argument must not be empty");
     }
+    env->permission()->Drop(env, scope, resource.ToStringView());
+    return;
   }
 
   env->permission()->Drop(env, scope, "");

test/parallel/test-permission-drop-env-empty-reference.js

@@ -0,0 +1,34 @@
+'use strict';
+
+const common = require('../common');
+const { isMainThread } = require('worker_threads');
+
+if (!isMainThread) {
+  common.skip('This test only works on a main thread');
+}
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+// An empty reference must not drop every environment variable.
+const { status, stdout, stderr } = spawnSync(process.execPath, [
+  '--permission',
+  '--allow-env=A,B',
+  '-e',
+  `
+    const assert = require('assert');
+    assert.throws(() => process.permission.drop('env', ''), {
+      code: 'ERR_INVALID_ARG_VALUE',
+    });
+    assert.throws(() => process.permission.drop('env', new Uint8Array(0)), {
+      code: 'ERR_INVALID_ARG_VALUE',
+    });
+    assert.strictEqual(process.env.A, '1');
+    assert.strictEqual(process.env.B, '2');
+  `,
+], {
+  env: { ...process.env, A: '1', B: '2' },
+  encoding: 'utf8',
+});
+
+assert.strictEqual(status, 0, `stdout: ${stdout}\nstderr: ${stderr}`);

test/parallel/test-permission-drop-errors.js

@@ -39,6 +39,25 @@ const assert = require('assert');
   }));
 }
 
+{
+  // An empty reference must not drop the whole scope.
+  for (const scope of ['fs.read', 'fs.write', 'child', 'env']) {
+    for (const reference of ['', new Uint8Array(0), Buffer.alloc(0)]) {
+      assert.throws(() => {
+        process.permission.drop(scope, reference);
+      }, common.expectsError({
+        code: 'ERR_INVALID_ARG_VALUE',
+      }));
+    }
+  }
+
+  assert.strictEqual(process.permission.has('fs.read'), true);
+  assert.strictEqual(process.permission.has('child'), false);
+
+  process.permission.drop('child', null);
+  process.permission.drop('child', undefined);
+}
+
 {
   process.permission.drop('invalid-scope');
   process.permission.drop('invalid-scope', '/tmp');