sea,vfs: address Joyee's code review feedback

- Rename unregisterVFS to deregisterVFS to match public API naming
- Fix Buffer copy safety in SEA provider: use Buffer.from(new
  Uint8Array(content)) to ensure returned buffers are independent
  copies safe to mutate, not views over read-only memory segments
- Consolidate C++ boolean getters (isSea, isVfsEnabled,
  isExperimentalSeaWarningNeeded) into boolean properties set once
  during Initialize(), avoiding repeated function call overhead
- Use Module.createRequire() when VFS is enabled in SEA so that
  require has all standard properties (resolve, cache, etc.) and
  builtin loading flows through hooks

Author: mcollina

lib/internal/main/embedding.js

@@ -12,10 +12,14 @@
 const {
   prepareMainThreadExecution,
 } = require('internal/process/pre_execution');
-const { isExperimentalSeaWarningNeeded, isSea, isVfsEnabled } = internalBinding('sea');
+const {
+  isSea: isLoadingSea,
+  isVfsEnabled: isVfsEnabledFlag,
+  isExperimentalSeaWarningNeeded,
+} = internalBinding('sea');
 const { emitExperimentalWarning } = require('internal/util');
 const { emitWarningSync } = require('internal/process/warning');
-const { Module, wrapModuleLoad } = require('internal/modules/cjs/loader');
+const { Module } = require('internal/modules/cjs/loader');
 const { compileFunctionForCJSLoader } = internalBinding('contextify');
 const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
 const { BuiltinModule } = require('internal/bootstrap/realm');
@@ -33,9 +37,8 @@ const path = require('path');
 // command line (e.g. it could be provided via an API or bundled into the executable).
 prepareMainThreadExecution(false, true);
 
-const isLoadingSea = isSea();
-const isBuiltinWarningNeeded = isLoadingSea && isExperimentalSeaWarningNeeded();
-if (isExperimentalSeaWarningNeeded()) {
+const isBuiltinWarningNeeded = isLoadingSea && isExperimentalSeaWarningNeeded;
+if (isExperimentalSeaWarningNeeded) {
   emitExperimentalWarning('Single executable application');
 }
 
@@ -86,14 +89,21 @@ function embedderRunCjs(content, filename) {
 
   // Patch the module to make it look almost like a regular CJS module
   // instance. When VFS is active, set the filename to a VFS path so that
-  // relative require('./foo.js') resolves under the VFS mount point.
+  // relative require('./foo.js') resolves under the VFS mount point,
+  // and use createRequire for a fully-featured require function.
+  let requireFn;
   if (seaVfsActive && seaVfsMountPoint) {
     customModule.filename = seaVfsMountPoint + '/' + path.basename(filename);
     customModule.paths = Module._nodeModulePaths(
       path.dirname(customModule.filename));
+    // Use createRequire so that require has all standard properties
+    // (resolve, cache, etc.) and builtin loading flows through hooks.
+    requireFn = Module.createRequire(customModule.filename);
+    requireFn.main = customModule;
   } else {
     customModule.filename = process.execPath;
     customModule.paths = Module._nodeModulePaths(process.execPath);
+    requireFn = embedderRequire;
   }
   embedderRequire.main = customModule;
 
@@ -104,7 +114,7 @@ function embedderRunCjs(content, filename) {
   // out parameter.
   return compiledWrapper(
     customModule.exports,  // exports
-    embedderRequire,       // require
+    requireFn,             // require
     customModule,          // module
     customModule.filename, // __filename
     path.dirname(customModule.filename),  // __dirname
@@ -134,7 +144,7 @@ function initSeaVfs() {
   if (seaVfsInitialized) return;
   seaVfsInitialized = true;
 
-  if (!isLoadingSea || !isVfsEnabled()) return;
+  if (!isLoadingSea || !isVfsEnabledFlag) return;
 
   // Check if SEA has VFS support
   const { getSeaVfs } = require('internal/vfs/sea');
@@ -148,18 +158,11 @@ function initSeaVfs() {
 function embedderRequire(id) {
   const normalizedId = normalizeRequirableId(id);
   if (normalizedId) {
-    // Built-in module
     return require(normalizedId);
   }
 
-  // When VFS is active, use wrapModuleLoad which flows through the
-  // registered Module.registerHooks({ resolve, load }) hooks in
-  // module_hooks.js, handling both absolute VFS paths and relative requires.
-  if (isLoadingSea && seaVfsActive) {
-    return wrapModuleLoad(id, embedderRequire.main, false);
-  }
-
-  // No VFS - show warning and throw
+  // When VFS is not active, only built-in modules are supported.
+  // VFS-enabled SEAs use createRequire instead of embedderRequire.
   warnNonBuiltinInSEA();
   throw new ERR_UNKNOWN_BUILTIN_MODULE(id);
 }

lib/internal/main/mksnapshot.js

@@ -202,7 +202,7 @@ function main() {
     return fn(requireForUserSnapshot, filename, dirname);
   }
 
-  if (isExperimentalSeaWarningNeeded()) {
+  if (isExperimentalSeaWarningNeeded) {
     emitExperimentalWarning('Single executable application');
   }
 

lib/internal/modules/helpers.js

@@ -132,7 +132,7 @@ let isSEABuiltinWarningNeeded_;
 function isSEABuiltinWarningNeeded() {
   if (isSEABuiltinWarningNeeded_ === undefined) {
     const { isExperimentalSeaWarningNeeded, isSea } = internalBinding('sea');
-    isSEABuiltinWarningNeeded_ = isSea() && isExperimentalSeaWarningNeeded();
+    isSEABuiltinWarningNeeded_ = isSea && isExperimentalSeaWarningNeeded;
   }
   return isSEABuiltinWarningNeeded_;
 }

lib/internal/vfs/file_system.js

@@ -75,13 +75,13 @@ const kOriginalCwd = Symbol('kOriginalCwd');
 
 // Lazy-loaded module hooks
 let registerVFS;
-let unregisterVFS;
+let deregisterVFS;
 
 function loadModuleHooks() {
   if (!registerVFS) {
     const hooks = require('internal/vfs/module_hooks');
     registerVFS = hooks.registerVFS;
-    unregisterVFS = hooks.unregisterVFS;
+    deregisterVFS = hooks.deregisterVFS;
   }
 }
 
@@ -290,7 +290,7 @@ class VirtualFileSystem {
     this.#unhookProcessCwd();
     if (this[kModuleHooks]) {
       loadModuleHooks();
-      unregisterVFS(this);
+      deregisterVFS(this);
     }
     this[kMountPoint] = null;
     this[kMounted] = false;

lib/internal/vfs/module_hooks.js

@@ -78,10 +78,10 @@ function registerVFS(vfs) {
 }
 
 /**
- * Unregisters a VFS instance.
+ * Deregisters a VFS instance.
  * @param {VirtualFileSystem} vfs The VFS instance to unregister
  */
-function unregisterVFS(vfs) {
+function deregisterVFS(vfs) {
   const index = ArrayPrototypeIndexOf(activeVFSList, vfs);
   if (index !== -1) {
     ArrayPrototypeSplice(activeVFSList, index, 1);
@@ -699,7 +699,7 @@ function installHooks() {
 
 module.exports = {
   registerVFS,
-  unregisterVFS,
+  deregisterVFS,
   findVFSForStat,
   findVFSForRead,
 };

lib/internal/vfs/providers/sea.js

@@ -147,7 +147,7 @@ class SEAProvider extends VirtualProvider {
     // Lazy-load SEA bindings
     const { isSea, getAsset, getAssetKeys } = internalBinding('sea');
 
-    if (!isSea()) {
+    if (!isSea) {
       throw new ERR_INVALID_STATE('SEAProvider can only be used in a Single Executable Application');
     }
 
@@ -252,7 +252,10 @@ class SEAProvider extends VirtualProvider {
       throw createENOENT('open', path);
     }
     const content = this[kGetAsset](key);
-    return Buffer.from(content);
+    // Create a copy so the returned buffer is independently mutable.
+    // Buffer.from(ArrayBuffer) creates a view sharing the same memory,
+    // which may be backed by read-only segments in the executable.
+    return Buffer.from(new Uint8Array(content));
   }
 
   openSync(path, flags, mode) {

lib/internal/vfs/sea.js

@@ -30,7 +30,7 @@ const lazySEAProvider = getLazy(
  * @returns {VirtualFileSystem|null} The VFS instance, or null if not running as SEA or no assets
  */
 function createSeaVfs(options = kEmptyObject) {
-  if (!isSea() || !isVfsEnabled()) {
+  if (!isSea || !isVfsEnabled) {
     return null;
   }
 

lib/sea.js

@@ -3,7 +3,7 @@ const {
   ArrayBufferPrototypeSlice,
 } = primordials;
 
-const { isSea, getAsset: getAssetInternal, getAssetKeys: getAssetKeysInternal } = internalBinding('sea');
+const { isSea: isSeaFlag, getAsset: getAssetInternal, getAssetKeys: getAssetKeysInternal } = internalBinding('sea');
 const { TextDecoder } = require('internal/encoding');
 const { validateString } = require('internal/validators');
 const {
@@ -23,7 +23,7 @@ const { Blob } = require('internal/blob');
 function getRawAsset(key) {
   validateString(key, 'key');
 
-  if (!isSea()) {
+  if (!isSeaFlag) {
     throw new ERR_NOT_IN_SINGLE_EXECUTABLE_APPLICATION();
   }
 
@@ -74,13 +74,17 @@ function getAssetAsBlob(key, options) {
  * @returns {string[]}
  */
 function getAssetKeys() {
-  if (!isSea()) {
+  if (!isSeaFlag) {
     throw new ERR_NOT_IN_SINGLE_EXECUTABLE_APPLICATION();
   }
 
   return getAssetKeysInternal() || [];
 }
 
+function isSea() {
+  return isSeaFlag;
+}
+
 module.exports = {
   isSea,
   getAsset,

src/node_sea.cc

@@ -258,37 +258,7 @@ SeaResource FindSingleExecutableResource() {
   return sea_resource;
 }
 
-void IsSea(const FunctionCallbackInfo<Value>& args) {
-  args.GetReturnValue().Set(IsSingleExecutable());
-}
-
-void IsVfsEnabled(const FunctionCallbackInfo<Value>& args) {
-  if (!IsSingleExecutable()) {
-    args.GetReturnValue().Set(false);
-    return;
-  }
-  SeaResource sea_resource = FindSingleExecutableResource();
-  args.GetReturnValue().Set(
-      static_cast<bool>(sea_resource.flags & SeaFlags::kEnableVfs));
-}
-
-void IsExperimentalSeaWarningNeeded(const FunctionCallbackInfo<Value>& args) {
-  bool is_building_sea =
-      !per_process::cli_options->experimental_sea_config.empty();
-  if (is_building_sea) {
-    args.GetReturnValue().Set(true);
-    return;
-  }
-
-  if (!IsSingleExecutable()) {
-    args.GetReturnValue().Set(false);
-    return;
-  }
-
-  SeaResource sea_resource = FindSingleExecutableResource();
-  args.GetReturnValue().Set(!static_cast<bool>(
-      sea_resource.flags & SeaFlags::kDisableExperimentalSeaWarning));
-}
+// Boolean flag getters removed - flags are set as properties in Initialize().
 
 std::tuple<int, char**> FixupArgsForSEA(int argc, char** argv) {
   // Repeats argv[0] at position 1 on argv as a replacement for the missing
@@ -909,20 +879,45 @@ void Initialize(Local<Object> target,
                 Local<Value> unused,
                 Local<Context> context,
                 void* priv) {
-  SetMethod(context, target, "isSea", IsSea);
-  SetMethod(context,
-            target,
-            "isExperimentalSeaWarningNeeded",
-            IsExperimentalSeaWarningNeeded);
-  SetMethod(context, target, "isVfsEnabled", IsVfsEnabled);
+  Isolate* isolate = context->GetIsolate();
+
+  // Set boolean flags as properties (computed once, avoids repeated calls).
+  bool is_sea = IsSingleExecutable();
+  bool is_vfs_enabled = false;
+  bool is_experimental_warning_needed =
+      !per_process::cli_options->experimental_sea_config.empty();
+
+  if (is_sea) {
+    SeaResource sea_resource = FindSingleExecutableResource();
+    is_vfs_enabled =
+        static_cast<bool>(sea_resource.flags & SeaFlags::kEnableVfs);
+    if (!static_cast<bool>(
+            sea_resource.flags & SeaFlags::kDisableExperimentalSeaWarning)) {
+      is_experimental_warning_needed = true;
+    }
+  }
+
+  target
+      ->Set(context,
+            FIXED_ONE_BYTE_STRING(isolate, "isSea"),
+            Boolean::New(isolate, is_sea))
+      .Check();
+  target
+      ->Set(context,
+            FIXED_ONE_BYTE_STRING(isolate, "isVfsEnabled"),
+            Boolean::New(isolate, is_vfs_enabled))
+      .Check();
+  target
+      ->Set(context,
+            FIXED_ONE_BYTE_STRING(isolate, "isExperimentalSeaWarningNeeded"),
+            Boolean::New(isolate, is_experimental_warning_needed))
+      .Check();
+
   SetMethod(context, target, "getAsset", GetAsset);
   SetMethod(context, target, "getAssetKeys", GetAssetKeys);
 }
 
 void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
-  registry->Register(IsSea);
-  registry->Register(IsExperimentalSeaWarningNeeded);
-  registry->Register(IsVfsEnabled);
   registry->Register(GetAsset);
   registry->Register(GetAssetKeys);
 }