Merge pull request #32 from jasnell/jasnell/node-bench-3

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

Author: jasnell

doc/api/bench.md

@@ -316,6 +316,9 @@ added: REPLACEME
 * `name` {string} The benchmark name. **Default:** The `name` property of `fn`,
   or `'<anonymous>'` when `fn` has no name.
 * `options` {Object}
+  * `diagnosticChannels` {Array} String diagnostics channel names, deduplicated
+    and inherited from containing suites by union. Symbol values in the array
+    are silently ignored. **Default:** `[]`.
   * `only` {boolean} When any benchmark or containing suite has `only` set,
     benchmarks without `only` in their hierarchy are skipped. **Default:**
     `false`.
@@ -346,6 +349,12 @@ samples, but their samples are discarded. An exception, rejection, timeout,
 abort, missing timing call, or duplicate timing call stops the current
 benchmark. Later benchmarks continue to run.
 
+For each warmup and measured callback, the runner subscribes to the configured
+diagnostics channels. Each publication queues a context diagnostic whose
+`message` is `{ name, message }`, containing the string channel name and the
+published message. Subscriptions are removed when the callback settles or is
+aborted.
+
 A timeout or abort cannot interrupt synchronous JavaScript and does not forcibly
 cancel asynchronous work that ignores `context.signal`.
 
@@ -388,6 +397,9 @@ added: REPLACEME
 * `name` {string} The suite name. **Default:** The `name` property of `fn`, or
   `'<anonymous>'` when `fn` has no name.
 * `options` {Object}
+  * `diagnosticChannels` {Array} String diagnostics channel names inherited by
+    nested suites and benchmarks. Symbol values in the array are silently
+    ignored. **Default:** `[]`.
   * `only` {boolean} Selects all benchmarks nested in this suite. **Default:**
     `false`.
   * `skip` {boolean|string} Skips all benchmarks nested in this suite.
@@ -504,7 +516,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 +530,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
@@ -659,7 +675,8 @@ message transport from the duration. `record()` is mutually exclusive with
 added: REPLACEME
 -->
 
-* `message` {string} The diagnostic message.
+* `message` {any} A structured-cloneable diagnostic value. With CLI process
+  isolation, it must also be supported by advanced child process serialization.
 * `options` {Object}
   * `level` {string} Either `'info'` or `'warning'`. **Default:** `'info'`.
   * `detail` {any} Additional structured-cloneable diagnostic data. With CLI
@@ -675,10 +692,10 @@ before a callback failure are emitted before the failed `'bench:complete'`
 event and do not themselves cause the benchmark to fail. If a timeout or abort
 wins before the callback settles, queued diagnostics might not be emitted.
 
-The message and options are validated, and detail is cloned, synchronously.
-Calling `diagnostic()` between `context.start()` and `context.end()` therefore
-includes that work in the measured duration. Invalid arguments or an
-uncloneable detail violate the sample contract.
+The message and detail are cloned synchronously. Options are also validated
+synchronously. Calling `diagnostic()` between `context.start()` and
+`context.end()` therefore includes that work in the measured duration. Invalid
+arguments or an uncloneable message or detail violate the sample contract.
 
 ### `context.done()`
 
@@ -747,6 +764,8 @@ isolation, all files share one runner and their plans are emitted before any
 benchmark executes. Plan data contains the benchmark-scoped identity, location,
 tags, and parameters described in [benchmark result][], together with:
 
+* `diagnosticChannels` {string\[]} The inherited string channel names
+  subscribed to during each callback.
 * `samples` {number} The effective maximum number of measured callback
   invocations after run-level overrides.
 * `warmup` {number} The effective number of unreported warmup callback

doc/api/cli.md

@@ -492,9 +492,7 @@ benchmark runner process. This reduces startup overhead but allows module,
 heap, and process state to carry between files. User writes to stdout or stderr
 also share destinations with benchmark reporters in this mode.
 
-The supported modes are `'process'` and `'none'`. Worker-thread isolation is not
-a CLI mode. Higher-level tools can implement it using externally measured
-samples as described in the [benchmark runner][] documentation.
+The supported modes are `'process'` and `'none'`.
 
 ### `--bench-name-pattern=pattern`
 

doc/node.1

@@ -304,6 +304,7 @@ When \fBmode\fR is \fB'none'\fR, all matching files and benchmarks run serially
 benchmark runner process. This reduces startup overhead but allows module,
 heap, and process state to carry between files. User writes to stdout or stderr
 also share destinations with benchmark reporters in this mode.
+The supported modes are \fB'process'\fR and \fB'none'\fR.
 .
 .It Fl -bench-name-pattern Ns = Ns Ar pattern
 Only runs benchmarks whose full hierarchical name matches the JavaScript

lib/internal/bench_runner/benchmark.js

@@ -48,6 +48,7 @@ const { structuredClone } = require('internal/worker/js_transferable');
 const { bigint: hrtime } = process.hrtime;
 const kDefaultSamples = 30;
 const kDefaultWarmup = 0;
+const kEmptyDiagnosticChannels = ObjectFreeze([]);
 const kEmptyNamePath = ObjectFreeze([]);
 const kEmptyParams = ObjectFreeze({ __proto__: null });
 const kEmptyTags = ObjectFreeze([]);
@@ -82,6 +83,33 @@ function canonicalizeTags(tags, parentTags = kEmptyTags) {
   return ObjectFreeze(result);
 }
 
+function canonicalizeDiagnosticChannels(
+  diagnosticChannels,
+  parentDiagnosticChannels = kEmptyDiagnosticChannels,
+) {
+  if (diagnosticChannels === undefined) return parentDiagnosticChannels;
+  if (!ArrayIsArray(diagnosticChannels)) {
+    throw new ERR_INVALID_ARG_TYPE(
+      'options.diagnosticChannels', 'Array', diagnosticChannels);
+  }
+
+  const result = ArrayPrototypeSlice(parentDiagnosticChannels);
+  const seen = new SafeSet(parentDiagnosticChannels);
+  for (let i = 0; i < diagnosticChannels.length; i++) {
+    const name = diagnosticChannels[i];
+    if (typeof name === 'symbol') continue;
+    if (typeof name !== 'string') {
+      throw new ERR_INVALID_ARG_TYPE(
+        `options.diagnosticChannels[${i}]`, ['string', 'symbol'], name);
+    }
+    if (!seen.has(name)) {
+      seen.add(name);
+      ArrayPrototypePush(result, name);
+    }
+  }
+  return ObjectFreeze(result);
+}
+
 function canonicalizeParams(params) {
   if (params === undefined) return kEmptyParams;
   validateObject(params, 'options.params');
@@ -106,15 +134,17 @@ function canonicalizeParams(params) {
   return ObjectFreeze(result);
 }
 
-function validateNodeOptions(options, parentTags) {
+function validateNodeOptions(options, parentTags, parentDiagnosticChannels) {
   validateObject(options, 'options');
-  const { only = false, skip, tags } = options;
+  const { diagnosticChannels, only = false, skip, tags } = options;
   if (typeof only !== 'boolean') {
     throw new ERR_INVALID_ARG_TYPE('options.only', 'boolean', only);
   }
   validateSkip(skip);
   return {
     __proto__: null,
+    diagnosticChannels: canonicalizeDiagnosticChannels(
+      diagnosticChannels, parentDiagnosticChannels),
     only,
     skip,
     tags: canonicalizeTags(tags, parentTags),
@@ -157,7 +187,10 @@ class Suite extends AsyncResource {
   constructor(harness, parent, name, options, fn, loc, isRoot = false) {
     super('BenchSuite');
     const validated = validateNodeOptions(
-      options, parent?.tags ?? kEmptyTags);
+      options,
+      parent?.tags ?? kEmptyTags,
+      parent?.diagnosticChannels ?? kEmptyDiagnosticChannels,
+    );
 
     this.harness = harness;
     this.parent = parent;
@@ -174,6 +207,7 @@ class Suite extends AsyncResource {
       this.namePath,
     ]);
     this.parentId = isRoot || parent.isRoot ? null : parent.suiteId;
+    this.diagnosticChannels = validated.diagnosticChannels;
     this.only = validated.only;
     this.skip = validated.skip;
     this.tags = validated.tags;
@@ -195,7 +229,8 @@ class Suite extends AsyncResource {
 class Bench extends AsyncResource {
   constructor(harness, parent, name, options, fn, loc) {
     super('Benchmark');
-    const validated = validateNodeOptions(options, parent.tags);
+    const validated = validateNodeOptions(
+      options, parent.tags, parent.diagnosticChannels);
     const {
       params,
       samples = kDefaultSamples,
@@ -216,6 +251,7 @@ class Bench extends AsyncResource {
     this.name = name;
     this.fn = fn;
     this.loc = createLocation(loc, harness.entryFile);
+    this.diagnosticChannels = validated.diagnosticChannels;
     this.only = validated.only;
     this.skip = validated.skip;
     this.tags = validated.tags;
@@ -275,15 +311,15 @@ class BenchContext {
       throw new ERR_INVALID_STATE('benchmark sample is no longer active');
     }
     try {
-      validateString(message, 'message');
+      const clonedMessage = structuredClone(message);
       validateObject(options, 'options');
       const { detail, level = 'info' } = options;
       validateString(level, 'options.level');
       if (level !== 'info' && level !== 'warning') {
         throw new ERR_INVALID_ARG_VALUE(
           'options.level', level, "must be 'info' or 'warning'");
       }
-      const diagnostic = { __proto__: null, level, message };
+      const diagnostic = { __proto__: null, level, message: clonedMessage };
       if (detail !== undefined) diagnostic.detail = structuredClone(detail);
       this.#onDiagnostic(diagnostic);
     } catch (error) {

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');
@@ -434,7 +440,8 @@ function validateRecord(record) {
       warmup,
       yieldBetweenSamples,
     } = record.data;
-    if (typeof record.data.file !== 'string' ||
+    if (!isStringArray(record.data.diagnosticChannels) ||
+        typeof record.data.file !== 'string' ||
         !NumberIsSafeInteger(record.data.line) || record.data.line < 0 ||
         !NumberIsSafeInteger(record.data.column) || record.data.column < 0 ||
         !isStringArray(record.data.tags) ||
@@ -465,7 +472,7 @@ function validateRecord(record) {
         record.data.phase !== 'measurement') ||
        !NumberIsSafeInteger(record.data.index) || record.data.index < 0 ||
        record.data.index > 0xFFFFFFFF ||
-       typeof record.data.message !== 'string' ||
+       !ObjectPrototypeHasOwnProperty(record.data, 'message') ||
        (record.data.level !== 'info' && record.data.level !== 'warning') ||
        typeof record.data.file !== 'string' ||
        !NumberIsSafeInteger(record.data.line) || record.data.line < 0 ||
@@ -758,6 +765,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 +1133,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 {