lib: have runFile honor permissions and accept URL/Buffer paths

Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode

Author: jasnell

doc/api/bench.md

@@ -504,7 +504,7 @@ for await (const { type, data } of run()) {
 added: REPLACEME
 -->
 
-* `path` {string} The absolute path of one benchmark module.
+* `path` {string|Buffer|URL} The path of one benchmark module.
 * `options` {Object}
   * `env` {Object} The child process environment. Property values must be
     strings or `undefined`. This replaces, rather than extends, the parent
@@ -518,18 +518,22 @@ added: REPLACEME
 * Returns: {BenchmarksStream}
 
 Runs exactly one benchmark module in a fresh child process and returns its
-object-mode event stream. `path` is not interpreted as a glob. Unless the signal
-is aborted or the stream is destroyed before startup, every call uses a new
-child. Input discovery, ordering, concurrency, retries, and multi-file
-scheduling remain the caller's responsibility.
+object-mode event stream. A relative `path` is resolved from the current working
+directory when `runFile()` is called. `path` is not interpreted as a glob.
+Unless the signal is aborted or the stream is destroyed before startup, every
+call uses a new child. Input discovery, ordering, concurrency, retries, and
+multi-file scheduling remain the caller's responsibility.
+
+When the Permission Model is enabled, the caller must have file system read
+access to `path` and permission to create child processes.
 
 Records use advanced child process serialization, preserving supported
 structured values such as `bigint` and errors. Child writes to stdout and stderr
-become `'bench:diagnostic'` records. A module loading error, abnormal child exit,
-or cancellation also emits an error diagnostic and produces a terminal
-`'bench:summary'` whose `success` property is `false`; these execution failures
-do not error the stream. If module evaluation fails after declaring benchmarks,
-those declarations still run before the unsuccessful summary.
+become `'bench:diagnostic'` records. A permission failure, module loading error,
+abnormal child exit, or cancellation also emits an error diagnostic and produces
+a terminal `'bench:summary'` whose `success` property is `false`; these execution
+failures do not error the stream. If module evaluation fails after declaring
+benchmarks, those declarations still run before the unsuccessful summary.
 
 `env`, effective inherited options, and an explicitly provided `execArgv` are
 copied when `runFile()` is called. The runner removes `NODE_OPTIONS`, replaces

lib/internal/bench_runner/cli.js

@@ -36,10 +36,14 @@ const {
   StringPrototypeStartsWith,
   StringPrototypeToUpperCase,
   SymbolDispose,
+  uncurryThis,
 } = primordials;
+const { Buffer } = require('buffer');
+const BufferToString = uncurryThis(Buffer.prototype.toString);
 const { spawn } = require('child_process');
 const { createWriteStream, statSync } = require('fs');
 const { Glob } = require('internal/fs/glob');
+const { getValidatedPath } = require('internal/fs/utils');
 const {
   BenchmarksStream,
 } = require('internal/bench_runner/benchmarks_stream');
@@ -53,6 +57,7 @@ const { deserializeError, serializeError } = require('internal/error_serdes');
 const {
   AbortError,
   codes: {
+    ERR_ACCESS_DENIED,
     ERR_INVALID_ARG_TYPE,
     ERR_INVALID_ARG_VALUE,
     ERR_INVALID_STATE,
@@ -64,6 +69,7 @@ const {
   getOptionValue,
   getOptionsAsFlagsFromBinding,
 } = require('internal/options');
+const permission = require('internal/process/permission');
 const { TIMEOUT_MAX } = require('internal/timers');
 const { kEmptyObject } = require('internal/util');
 const {
@@ -75,7 +81,7 @@ const {
 } = require('internal/validators');
 const { pathToFileURL } = require('internal/url');
 const { pipeline } = require('stream/promises');
-const { isAbsolute, resolve, sep } = require('path');
+const { resolve, sep } = require('path');
 const { clearTimeout, setTimeout } = require('timers');
 
 const console = require('internal/console/global');
@@ -758,6 +764,16 @@ async function runChild(path, options, scope, onRecord) {
       }),
     };
   }
+  const resource = resolve(options.cwd, path);
+  if (permission.isEnabled() &&
+      !permission.has('fs.read', resource) &&
+      !permission.isAuditMode()) {
+    throw new ERR_ACCESS_DENIED(
+      'Access to this API has been restricted. Use --allow-fs-read to manage permissions.',
+      'FileSystemRead',
+      resource,
+    );
+  }
   const child = spawn(
     options.execPath ?? process.execPath,
     getChildArgs(path, options),
@@ -1116,10 +1132,8 @@ async function runIsolated(files, options, output) {
 }
 
 function runFile(path, options = kEmptyObject) {
-  validateStringWithoutNullBytes(path, 'path');
-  if (!isAbsolute(path)) {
-    throw new ERR_INVALID_ARG_VALUE('path', path, 'must be an absolute path');
-  }
+  path = getValidatedPath(path);
+  if (typeof path !== 'string') path = BufferToString(path);
   const file = resolve(path);
   validateObject(options, 'options');
   const {

test/parallel/test-bench-run-file.js

@@ -10,9 +10,9 @@ const path = require('path');
 const { runFile } = require('node:bench');
 
 const fixture = fixtures.path('bench-runner/run-file.cjs');
+const relativeFixture = path.relative(process.cwd(), fixture);
 
 assert.throws(() => runFile(null), { code: 'ERR_INVALID_ARG_TYPE' });
-assert.throws(() => runFile('relative.cjs'), { code: 'ERR_INVALID_ARG_VALUE' });
 assert.throws(() => runFile(fixture, null), { code: 'ERR_INVALID_ARG_TYPE' });
 assert.throws(() => runFile(fixture, { execArgv: null }), {
   code: 'ERR_INVALID_ARG_TYPE',
@@ -50,6 +50,9 @@ assert.throws(() => runFile(fixture, {
 assert.throws(() => runFile(`${fixture}\0`), {
   code: 'ERR_INVALID_ARG_VALUE',
 });
+assert.throws(() => runFile(Buffer.from(`${fixture}\0`)), {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
 assert.throws(() => runFile(fixture, { env: null }), {
   code: 'ERR_INVALID_ARG_TYPE',
 });
@@ -79,7 +82,7 @@ async function testRunFile() {
     NODE_CHANNEL_FD: '999',
     NODE_CHANNEL_SERIALIZATION_MODE: 'json',
   };
-  const stream = runFile(fixture, { env, execArgv });
+  const stream = runFile(relativeFixture, { env, execArgv });
   execArgv.length = 0;
   env.NODE_BENCH_RUN_FILE = 'mutated';
   const records = await stream.toArray();
@@ -108,8 +111,11 @@ async function testRunFile() {
 
 async function testConcurrentCalls() {
   const [cjsRecords, esmRecords] = await Promise.all([
-    runFile(fixtures.path('bench-runner/a.cjs')).toArray(),
-    runFile(fixtures.path('bench-runner/b.mjs')).toArray(),
+    runFile(fixtures.fileURL('bench-runner/a.cjs')).toArray(),
+    runFile(Buffer.from(path.relative(
+      process.cwd(),
+      fixtures.path('bench-runner/b.mjs'),
+    ))).toArray(),
   ]);
   const cjsResult = cjsRecords.find(
     ({ type }) => type === 'bench:complete').data;
@@ -119,6 +125,14 @@ async function testConcurrentCalls() {
   assert.strictEqual(esmResult.name, 'beta');
   assert.notStrictEqual(cjsResult.params.pid, esmResult.params.pid);
   assert.notStrictEqual(cjsResult.runId, esmResult.runId);
+  assert.strictEqual(
+    cjsRecords.at(-1).data.file,
+    fixtures.path('bench-runner/a.cjs'),
+  );
+  assert.strictEqual(
+    esmRecords.at(-1).data.file,
+    fixtures.path('bench-runner/b.mjs'),
+  );
 }
 
 async function testLoadFailure() {
@@ -280,6 +294,74 @@ function testEvalParent() {
   assert.strictEqual(result.stdout, 'true\n');
 }
 
+function testPermissions() {
+  const fsReadScript = `
+    const assert = require('assert');
+    const { runFile } = require('node:bench');
+    const { pathToFileURL } = require('url');
+    const target = ${JSON.stringify(fixture)};
+    assert.strictEqual(process.permission.has('child'), true);
+    assert.strictEqual(process.permission.has('fs.read', target), false);
+    Promise.all([
+      target,
+      Buffer.from(target),
+      pathToFileURL(target),
+    ].map(async (input) => {
+      const records = await runFile(input).toArray();
+      const diagnostic = records.find(({ type, data }) =>
+        type === 'bench:diagnostic' &&
+        data.error?.code === 'ERR_ACCESS_DENIED');
+      assert.strictEqual(diagnostic.data.error.permission, 'FileSystemRead');
+      assert.strictEqual(diagnostic.data.error.resource, target);
+      assert.strictEqual(records.at(-1).type, 'bench:summary');
+      assert.strictEqual(records.at(-1).data.success, false);
+    })).catch((error) => {
+      console.error(error);
+      process.exitCode = 1;
+    });
+  `;
+  const result = spawnSync(process.execPath, [
+    '--no-warnings',
+    '--permission',
+    '--allow-child-process',
+    '-e',
+    fsReadScript,
+  ], { encoding: 'utf8' });
+  assert.strictEqual(result.status, 0, result.stderr);
+
+  const childProcessScript = `
+    const assert = require('assert');
+    const { runFile } = require('node:bench');
+    const target = ${JSON.stringify(fixture)};
+    assert.strictEqual(process.permission.has('fs.read', target), true);
+    assert.strictEqual(process.permission.has('child'), false);
+    runFile(target).toArray().then((records) => {
+      const diagnostic = records.find(({ type, data }) =>
+        type === 'bench:diagnostic' &&
+        data.error?.code === 'ERR_ACCESS_DENIED');
+      assert.strictEqual(diagnostic.data.error.permission, 'ChildProcess');
+      assert.strictEqual(diagnostic.data.error.resource, process.execPath);
+      assert.strictEqual(records.at(-1).type, 'bench:summary');
+      assert.strictEqual(records.at(-1).data.success, false);
+    }).catch((error) => {
+      console.error(error);
+      process.exitCode = 1;
+    });
+  `;
+  const childProcessResult = spawnSync(process.execPath, [
+    '--no-warnings',
+    '--permission',
+    `--allow-fs-read=${fixture}`,
+    '-e',
+    childProcessScript,
+  ], { encoding: 'utf8' });
+  assert.strictEqual(
+    childProcessResult.status,
+    0,
+    childProcessResult.stderr,
+  );
+}
+
 async function testPreAborted() {
   const records = await runFile(fixture, {
     signal: AbortSignal.abort(new Error('already cancelled')),
@@ -315,4 +397,5 @@ async function testDestroy() {
   await testPreAborted();
   await testDestroy();
   testEvalParent();
+  testPermissions();
 })().then(common.mustCall());