Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow per-module choice for vm context #521

Merged
merged 7 commits into from May 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -142,6 +142,7 @@ Unlike `VM`, `NodeVM` allows you to require modules in the same way that you wou
* `require.root` - Restricted path(s) where local modules can be required (default: every path).
* `require.mock` - Collection of mock modules (both external or built-in).
* `require.context` - `host` (default) to require modules in the host and proxy them into the sandbox. `sandbox` to load, compile, and require modules in the sandbox. Except for `events`, built-in modules are always required in the host and proxied into the sandbox.
* `require.pathContext` - A callback allowing custom context to be determined per module. Parameters are the module name, and extension. The callback must return `host` or `sandbox` as per above.
* `require.import` - An array of modules to be loaded into NodeVM on start.
* `require.resolve` - An additional lookup function in case a module wasn't found in one of the traditional node lookup paths.
* `require.customRequire` - Use instead of the `require` function to load modules from the host.
Expand Down
15 changes: 14 additions & 1 deletion lib/nodevm.js
Expand Up @@ -17,6 +17,17 @@
* @return {*} The required module object.
*/

/**
* This callback will be called to specify the context to use "per" module.
*
* NOTE: many interoperating modules must live in the same context.
*
* @callback pathContextCallback
* @param {string} moduleName - Name of the module requested.
* @param {string} extensionType - The type of extension (node, js, json)
* @return {("host"|"sandbox")} The context for this module.
*/

const fs = require('fs');
const pa = require('path');
const {
Expand Down Expand Up @@ -180,11 +191,13 @@ class NodeVM extends VM {
* @param {("host"|"sandbox")} [options.require.context="host"] - <code>host</code> to require modules in host and proxy them to sandbox.
* <code>sandbox</code> to load, compile and require modules in sandbox.
* Builtin modules except <code>events</code> always required in host and proxied to sandbox.
* @param {pathContextCallback} [options.require.pathContext] - A callback per-module path to customize "where" to load a module.
* Builtin modules except <code>events</code> always required in host and proxied to sandbox.
* @param {string[]} [options.require.import] - Array of modules to be loaded into NodeVM on start.
* @param {resolveCallback} [options.require.resolve] - An additional lookup function in case a module wasn't
* found in one of the traditional node lookup paths.
* @param {customRequire} [options.require.customRequire=require] - Custom require to require host and built-in modules.
* @param {boolean} [option.require.strict=true] - Load required modules in strict mode.
* @param {boolean} [options.require.strict=true] - Load required modules in strict mode.
* @param {boolean} [options.nesting=false] -
* <b>WARNING: Allowing this is a security risk as scripts can create a NodeVM which can require any host module.</b>
* Allow nesting of VMs.
Expand Down
6 changes: 4 additions & 2 deletions lib/resolver-compat.js
Expand Up @@ -291,6 +291,8 @@ function resolverFromOptions(vm, options, override, compiler) {

const builtins = genBuiltinsFromOptions(vm, builtinOpt, mockOpt, override);

const pathContext = options.pathContext || (() => context);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to pass the pathContext function through the context option so that the context option then allows to be "host" | "sandbox" | pathContextCallback.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's definitely a cleaner option. Will change.


if (!externalOpt) return new Resolver(fsOpt, builtins, [], hostRequire);

let checkPath;
Expand Down Expand Up @@ -333,7 +335,7 @@ function resolverFromOptions(vm, options, override, compiler) {
}

if (typeof externalOpt !== 'object') {
return new DefaultResolver(fsOpt, builtins, checkPath, [], () => context, newCustomResolver, hostRequire, compiler, strict);
return new DefaultResolver(fsOpt, builtins, checkPath, [], pathContext, newCustomResolver, hostRequire, compiler, strict);
}

let transitive = false;
Expand All @@ -344,7 +346,7 @@ function resolverFromOptions(vm, options, override, compiler) {
transitive = context === 'sandbox' && externalOpt.transitive;
}
externals = external.map(makeExternalMatcher);
return new LegacyResolver(fsOpt, builtins, checkPath, [], () => context, newCustomResolver, hostRequire, compiler, strict, externals, transitive);
return new LegacyResolver(fsOpt, builtins, checkPath, [], pathContext, newCustomResolver, hostRequire, compiler, strict, externals, transitive);
}

exports.resolverFromOptions = resolverFromOptions;
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.