diff --git a/doc/api/permissions.md b/doc/api/permissions.md index 386671b339dc..e1ddb93e2412 100644 --- a/doc/api/permissions.md +++ b/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 diff --git a/doc/api/process.md b/doc/api/process.md index ba3832ccb855..15fb14f715e8 100644 --- a/doc/api/process.md +++ b/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: diff --git a/lib/internal/process/permission.js b/lib/internal/process/permission.js index 92e54fbab493..87ab0c65b663 100644 --- a/lib/internal/process/permission.js +++ b/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) { diff --git a/src/permission/permission.cc b/src/permission/permission.cc index d5e93c729be0..4482a530da5b 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -116,15 +116,18 @@ static void Drop(const FunctionCallbackInfo& 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, ""); diff --git a/test/parallel/test-permission-drop-env-empty-reference.js b/test/parallel/test-permission-drop-env-empty-reference.js new file mode 100644 index 000000000000..924218a9112e --- /dev/null +++ b/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}`); diff --git a/test/parallel/test-permission-drop-errors.js b/test/parallel/test-permission-drop-errors.js index ae3a9b2fcc9e..35765690bc58 100644 --- a/test/parallel/test-permission-drop-errors.js +++ b/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');