vfs: keep --experimental-vfs negatable

Keep VFS enabled by default while allowing the negated flag to disable
node:vfs exposure and --vfs-load.

Assisted-by: pi
Signed-off-by: Matteo Collina <hello@matteocollina.com>

Author: mcollina

doc/api/cli.md

@@ -2313,6 +2313,18 @@ changes:
 
 Disable the experimental [`node:sqlite`][] module.
 
+### `--no-experimental-vfs`
+
+<!-- YAML
+added: v26.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/66318
+    description: VFS is unflagged but still experimental.
+-->
+
+Disable the experimental [`node:vfs`][] module and `--vfs-load`.
+
 ### `--no-experimental-webstorage`
 
 <!-- YAML
@@ -3853,7 +3865,9 @@ May be given at most once.
 
 Mounts `source` as a virtual file system ([`node:vfs`][]), and runs the entry
 point and all subsequent `require()`/`import` resolution against that mount
-rather than the real file system. The mount is placed at a reserved mount point
+rather than the real file system. This option is unavailable when
+`--no-experimental-vfs` disables [`node:vfs`][]. The mount is placed at a
+reserved mount point
 assigned by Node.js, so it never shadows real paths and no target can be
 chosen. The entry point is taken from the mount the same way `node <directory>`
 takes one: the mount's own `package.json` `"main"`, or `index.js`. Any
@@ -4255,6 +4269,7 @@ one is included in the list below.
 * `--no-experimental-global-navigator`
 * `--no-experimental-sqlite`
 * `--no-experimental-strip-types`
+* `--no-experimental-vfs`
 * `--no-experimental-webstorage`
 * `--no-extra-info-on-fatal-exception`
 * `--no-force-async-hooks-checks`

doc/node.1

@@ -1194,6 +1194,9 @@ Legacy alias for \fB--no-require-module\fR.
 .It Fl -no-experimental-sqlite
 Disable the experimental \fBnode:sqlite\fR module.
 .
+.It Fl -no-experimental-vfs
+Disable the experimental \fBnode:vfs\fR module and \fB--vfs-load\fR.
+.
 .It Fl -no-experimental-webstorage
 Disable \fBWeb Storage\fR support.
 .
@@ -1932,7 +1935,9 @@ Print node's version.
 May be given at most once.
 Mounts \fBsource\fR as a virtual file system (\fBnode:vfs\fR), and runs the entry
 point and all subsequent \fBrequire()\fR/\fBimport\fR resolution against that mount
-rather than the real file system. The mount is placed at a reserved mount point
+rather than the real file system. This option is unavailable when
+\fB--no-experimental-vfs\fR disables \fBnode:vfs\fR. The mount is placed at a
+reserved mount point
 assigned by Node.js, so it never shadows real paths and no target can be
 chosen. The entry point is taken from the mount the same way \fBnode <directory>\fR
 takes one: the mount's own \fBpackage.json\fR \fB"main"\fR, or \fBindex.js\fR. Any
@@ -2294,6 +2299,8 @@ one is included in the list below.
 .It
 \fB--no-experimental-strip-types\fR
 .It
+\fB--no-experimental-vfs\fR
+.It
 \fB--no-experimental-webstorage\fR
 .It
 \fB--no-extra-info-on-fatal-exception\fR

lib/internal/process/pre_execution.js

@@ -524,6 +524,10 @@ function setupQuic() {
 }
 
 function setupVfs() {
+  if (!getOptionValue('--experimental-vfs')) {
+    return;
+  }
+
   const { BuiltinModule } = require('internal/bootstrap/realm');
   BuiltinModule.allowRequireByUsers('vfs');
 }

src/node.cc

@@ -1049,6 +1049,9 @@ static ExitCode InitializeNodeWithArgsInternal(
   // count below needs the arguments the command line itself gave.
   {
     auto* env_options = per_process::cli_options->per_isolate->per_env.get();
+    if (!env_options->experimental_vfs && env_options->vfs_load) {
+      errors->push_back("--vfs-load requires node:vfs to be enabled");
+    }
     // A second --vfs-load would silently replace the first, and the option
     // itself cannot say how often it was given; count it in the node options
     // the command line yielded.

src/node_options.cc

@@ -789,11 +789,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             NoOp{},
 #endif
             kAllowedInEnvvar);
-  // Retained as a no-op for compatibility; node:vfs is no longer gated.
   AddOption("--experimental-vfs",
-            "",
+            "experimental node:vfs module",
             BOOL_FIELD(experimental_vfs),
-            kAllowedInEnvvar);
+            kAllowedInEnvvar,
+            true);
   // Choosing the entry point is the command line's alone: an environment
   // variable must not be able to redirect what a `node <args>` invocation runs,
   // so this is rejected in NODE_OPTIONS. The source and whether to run from it

src/node_options.h

@@ -214,7 +214,7 @@ class EnvironmentOptions : public Options {
   DEFINE_BOOL_FIELD(experimental_websocket) = true;
   DEFINE_BOOL_FIELD(experimental_sqlite) = HAVE_SQLITE;
   DEFINE_BOOL_FIELD(experimental_stream_iter) = EXPERIMENTALS_DEFAULT_VALUE;
-  DEFINE_BOOL_FIELD(experimental_vfs) = EXPERIMENTALS_DEFAULT_VALUE;
+  DEFINE_BOOL_FIELD(experimental_vfs) = true;
   DEFINE_BOOL_FIELD(vfs_load) = false;
   DEFINE_BOOL_FIELD(webstorage) = HAVE_SQLITE;
   DEFINE_BOOL_FIELD(experimental_dtls) = EXPERIMENTALS_DEFAULT_VALUE;

test/parallel/test-module-builtin-experimental.js

@@ -51,6 +51,13 @@ if (common.hasFFI) {
   ], { status: 0 });
 }
 
+// node:vfs is enabled by default and can be disabled with
+// --no-experimental-vfs.
+spawnSyncAndAssert(process.execPath, [
+  '--no-experimental-vfs',
+  '-e', `const m = require('node:module'); if (m.builtinModules.includes('node:vfs')) process.exit(1); try { require('node:vfs'); } catch (e) { if (e.code === 'ERR_UNKNOWN_BUILTIN_MODULE') process.exit(0); } process.exit(1);`,
+], { status: 0 });
+
 const schemeOnlyBuiltins = ['node:test', 'node:sea', 'node:vfs'];
 if (common.hasFFI) {
   schemeOnlyBuiltins.push('node:ffi');

test/parallel/test-vfs-flag.js

@@ -1,7 +1,7 @@
 'use strict';
 
-// node:vfs is available without an experimental flag. The old flag remains a
-// no-op for compatibility, and the module remains available with --no- form.
+// node:vfs is available without an experimental flag. The old positive flag
+// remains accepted for compatibility, and --no-experimental-vfs disables it.
 
 require('../common');
 const { spawnSyncAndAssert } = require('../common/child_process');
@@ -28,14 +28,25 @@ const { spawnSyncAndAssert } = require('../common/child_process');
   });
 }
 
-// The old flag is accepted for compatibility but no longer gates the module.
-for (const flag of ['--experimental-vfs', '--no-experimental-vfs']) {
-  spawnSyncAndAssert(process.execPath, [
-    flag,
-    '-p',
-    'require("node:module").builtinModules.includes("node:vfs")',
-  ], { stdout: 'true\n', stderr: '' });
-}
+// The old positive flag is accepted for compatibility but no longer gates the
+// module.
+spawnSyncAndAssert(process.execPath, [
+  '--experimental-vfs',
+  '-p',
+  'require("node:module").builtinModules.includes("node:vfs")',
+], { stdout: 'true\n', stderr: '' });
+
+// --no-experimental-vfs disables node:vfs.
+spawnSyncAndAssert(process.execPath, [
+  '--no-experimental-vfs',
+  '-p',
+  'require("node:module").builtinModules.includes("node:vfs")',
+], { stdout: 'false\n', stderr: '' });
+
+spawnSyncAndAssert(process.execPath, [
+  '--no-experimental-vfs',
+  '-e', 'require("node:vfs")',
+], { status: 1, stderr: /ERR_UNKNOWN_BUILTIN_MODULE/ });
 
 // Bare `vfs` (no node: scheme) remains unavailable.
 spawnSyncAndAssert(process.execPath, [

test/parallel/test-vfs-load.js

@@ -49,6 +49,15 @@ function envArg(flag, value) {
   assert.match(res.stdout, /hello from inside the mount/);
 }
 
+// --no-experimental-vfs disables --vfs-load.
+{
+  const dir = fixture('disabled');
+  fs.mkdirSync(dir, { recursive: true });
+  const res = run(['--no-experimental-vfs', `--vfs-load=${dir}`]);
+  assert.notStrictEqual(res.status, 0);
+  assert.match(res.stderr, /--vfs-load requires node:vfs to be enabled/);
+}
+
 // A provider registered by a -r (CommonJS) preload backs a custom file format.
 {
   const providerModule = fixture('provider.js');
@@ -318,8 +327,8 @@ if (hasNodeOptions) {
   }
 }
 
-// The old --experimental-vfs option remains accepted in NODE_OPTIONS, but no
-// longer gates --vfs-load.
+// The old --experimental-vfs option remains accepted in NODE_OPTIONS. Its
+// positive form no longer gates --vfs-load, and its negated form disables it.
 if (hasNodeOptions) {
   const dir = fixture('env-flag-cli-load');
   fs.mkdirSync(dir, { recursive: true });
@@ -331,6 +340,13 @@ if (hasNodeOptions) {
   });
   assert.strictEqual(res.status, 0, res.stderr);
   assert.match(res.stdout, /ran/);
+
+  const disabled = spawnSync(process.execPath, [`--vfs-load=${dir}`], {
+    encoding: 'utf8',
+    env: { ...process.env, NODE_OPTIONS: '--no-experimental-vfs' },
+  });
+  assert.notStrictEqual(disabled.status, 0);
+  assert.match(disabled.stderr, /--vfs-load requires node:vfs to be enabled/);
 }
 
 // Under --vfs-load the entry point comes from the mount, so no positional