diff --git a/lib/internal/process/permission.js b/lib/internal/process/permission.js index 8dd92b1709b2..acfbab036bbb 100644 --- a/lib/internal/process/permission.js +++ b/lib/internal/process/permission.js @@ -3,17 +3,44 @@ const { ArrayPrototypePush, ObjectFreeze, + StringPrototypeStartsWith, } = primordials; const permission = internalBinding('permission'); -const { validateString, validateBuffer } = require('internal/validators'); -const { Buffer } = require('buffer'); -const { isBuffer } = Buffer; +const { validateString } = require('internal/validators'); +const { isUint8Array } = require('internal/util/types'); +const { isURL, fileURLToPath } = require('internal/url'); let _permission; let _audit; let _ffi; +function normalizeReference(scope, reference, name) { + if (reference == null) { + return reference; + } + + if (isURL(reference)) { + // Reference is only meaningful for fs.* scopes (FSPermission does + // per-path checks); every other scope ignores it (BooleanPermission), + // so only fs.* scopes require a file: URL. + if (StringPrototypeStartsWith(scope, 'fs')) { + return fileURLToPath(reference); + } + return reference.href; + } + + if (isUint8Array(reference)) { + // Passed through as-is: the native binding copies the raw bytes + // directly instead of forcing a (potentially lossy) UTF-8 string + // conversion, since paths are not guaranteed to be valid UTF-8. + return reference; + } + + validateString(reference, name); + return reference; +} + module.exports = ObjectFreeze({ __proto__: null, isEnabled() { @@ -32,28 +59,11 @@ module.exports = ObjectFreeze({ }, has(scope, reference) { validateString(scope, 'scope'); - if (reference != null) { - // TODO: add support for WHATWG URLs and Uint8Arrays. - if (isBuffer(reference)) { - validateBuffer(reference, 'reference'); - } else { - validateString(reference, 'reference'); - } - } - - return permission.has(scope, reference); + return permission.has(scope, normalizeReference(scope, reference, 'reference')); }, drop(scope, reference) { validateString(scope, 'scope'); - if (reference != null) { - if (isBuffer(reference)) { - validateBuffer(reference, 'reference'); - } else { - validateString(reference, 'reference'); - } - } - - permission.drop(scope, reference); + permission.drop(scope, normalizeReference(scope, reference, 'reference')); }, availableFlags() { if (_ffi === undefined) { diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 29449c0ec028..1fa26758e327 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -90,9 +90,12 @@ static void Drop(const FunctionCallbackInfo& args) { } if (args.Length() > 1 && !args[1]->IsUndefined()) { - Utf8Value utf8_arg(env->isolate(), args[1]); - if (utf8_arg.length() > 0) { - env->permission()->Drop(env, scope, utf8_arg.ToStringView()); + // 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; } } @@ -113,13 +116,16 @@ static void Has(const FunctionCallbackInfo& args) { } if (args.Length() > 1 && !args[1]->IsUndefined()) { - Utf8Value utf8_arg(env->isolate(), args[1]); - if (utf8_arg.length() == 0) { + // 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) { args.GetReturnValue().Set(false); return; } return args.GetReturnValue().Set( - env->permission()->is_granted(env, scope, utf8_arg.ToStringView())); + env->permission()->is_granted(env, scope, resource.ToStringView())); } return args.GetReturnValue().Set(env->permission()->is_granted(env, scope)); @@ -174,16 +180,22 @@ static bool FastHasResource( return false; } - Local res_str; - if (!resource_arg->ToString(context).ToLocal(&res_str)) { - return false; + // The JS wrapper always calls this with 2 arguments, passing undefined + // when no resource was given, so this path (unlike Has()) has to check + // for that explicitly and fall back to the scope-only check. + if (resource_arg->IsUndefined()) { + return env->permission()->is_granted(env, scope); } - Utf8Value utf8_res(isolate, res_str); - if (utf8_res.length() == 0) { + + // BufferValue is constructed directly from resource_arg (not from a + // pre-converted String) so that a Buffer/TypedArray's raw bytes are + // copied as-is instead of going through a lossy UTF-8 string conversion. + BufferValue resource(isolate, resource_arg); + if (resource.length() == 0) { return false; } - return env->permission()->is_granted(env, scope, utf8_res.ToStringView()); + return env->permission()->is_granted(env, scope, resource.ToStringView()); } static CFunction fast_has_methods_[] = {CFunction::Make(FastHas), diff --git a/test/fixtures/permission/has-reference-types.js b/test/fixtures/permission/has-reference-types.js new file mode 100644 index 000000000000..191d8c439604 --- /dev/null +++ b/test/fixtures/permission/has-reference-types.js @@ -0,0 +1,41 @@ +'use strict'; + +const assert = require('assert'); +const { pathToFileURL } = require('url'); + +const allowedDir = process.env.ALLOWED_DIR; +const allowedFile = process.env.ALLOWED_FILE; +const deniedFile = process.env.DENIED_FILE; + +// file: URL +{ + assert.strictEqual(process.permission.has('fs.read', pathToFileURL(allowedFile)), true); + assert.strictEqual(process.permission.has('fs.read', pathToFileURL(deniedFile)), false); + assert.throws(() => { + process.permission.has('fs.read', new URL('https://example.com')); + }, { code: 'ERR_INVALID_URL_SCHEME' }); +} + +// Non-fs scopes like net ignore the reference, so a non-file: URL must not be rejected here. +{ + assert.strictEqual(process.permission.has('net', new URL('https://example.com')), false); +} + +// Uint8Array +{ + const allowedBytes = new Uint8Array(Buffer.from(allowedFile)); + const deniedBytes = new Uint8Array(Buffer.from(deniedFile)); + assert.strictEqual(process.permission.has('fs.read', allowedBytes), true); + assert.strictEqual(process.permission.has('fs.read', deniedBytes), false); + + // Must respect byteOffset/byteLength, not read from the start of the buffer. + const padded = Buffer.concat([Buffer.from('padding-'), Buffer.from(allowedFile)]); + const view = new Uint8Array(padded.buffer, padded.byteOffset + 8, allowedFile.length); + assert.strictEqual(process.permission.has('fs.read', view), true); +} + +// drop() only matches the exact granted path (the directory), not a file inside it. +{ + process.permission.drop('fs.read', pathToFileURL(allowedDir)); + assert.strictEqual(process.permission.has('fs.read', pathToFileURL(allowedFile)), false); +} diff --git a/test/parallel/test-permission-has-reference-types.js b/test/parallel/test-permission-has-reference-types.js new file mode 100644 index 000000000000..8509046dc5ee --- /dev/null +++ b/test/parallel/test-permission-has-reference-types.js @@ -0,0 +1,48 @@ +'use strict'; + +// Test that process.permission.has()/drop() accept a URL or Uint8Array reference. + +require('../common'); +const { spawnSync } = require('child_process'); +const assert = require('assert'); +const { isMainThread } = require('worker_threads'); + +if (!isMainThread) { + process.exit(0); +} + +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); +const fs = require('fs'); +const path = require('path'); + +tmpdir.refresh(); + +const allowedDir = path.join(tmpdir.path, 'allowed'); +const deniedDir = path.join(tmpdir.path, 'denied'); +fs.mkdirSync(allowedDir); +fs.mkdirSync(deniedDir); + +const allowedFile = path.join(allowedDir, 'a.txt'); +const deniedFile = path.join(deniedDir, 'b.txt'); +fs.writeFileSync(allowedFile, 'allowed'); +fs.writeFileSync(deniedFile, 'denied'); + +const { status, stderr } = spawnSync( + process.execPath, + [ + '--permission', + `--allow-fs-read=${allowedDir}`, + fixtures.path('permission', 'has-reference-types.js'), + ], + { + env: { + ...process.env, + ALLOWED_DIR: allowedDir, + ALLOWED_FILE: allowedFile, + DENIED_FILE: deniedFile, + }, + }, +); + +assert.strictEqual(status, 0, stderr.toString());