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

Change in import behavior with custom "resolve" function in 3.9.6 #398

Closed
bespokebob opened this issue Feb 9, 2022 · 8 comments
Closed

Comments

@bespokebob
Copy link
Contributor

bespokebob commented Feb 9, 2022

If a custom "resolve" function is provided, it seems to be called even if the module is not in the allowed list of external modules. This is a change from previous versions.

const { NodeVM } = require('vm2')

const vm = new NodeVM({
  require: {
    external: ['jose'],
    resolve: require.resolve,
  },
})

vm.run("var jmespath = require('jmespath')", 'vm.js')

In 3.9.5, this code throws VMError [Error]: The module 'jmespath' is not whitelisted in VM. In 3.9.6, it passes without error.

@XmiliaH
Copy link
Collaborator

XmiliaH commented Feb 9, 2022

Relative path requires did allow to require any module under options.root circumventing the whitelist altogether. Therefore, the whitelist check was moved to a later point in time.

@bespokebob
Copy link
Contributor Author

It seems like the check isn't actually being hit now, though. If the custom 'resolve' function returns a path, it isn't validated against the list of external modules. I have to duplicate the check for allowed modules in my custom resolve function to get the expected behavior.

@XmiliaH
Copy link
Collaborator

XmiliaH commented Feb 9, 2022

The access check is done when trying to access files through

isPathAllowed(path) {
.

@bespokebob
Copy link
Contributor Author

I think this line is the culprit:

if (externals) externals.push(new RegExp('^' + escapeRegExp(resolved)));

If the custom resolver returns a path, it gets automatically added to the "external" list. Removing that line restores the previous behavior.

@XmiliaH
Copy link
Collaborator

XmiliaH commented Feb 9, 2022

Thanks, I see the problem now.

@XmiliaH
Copy link
Collaborator

XmiliaH commented Feb 10, 2022

Would you mind to check if the behavior with #400 is as expected.

@bespokebob
Copy link
Contributor Author

Would you mind to check if the behavior with #400 is as expected.

#400 fixes the issue for me. The exact error message returned is different, but that's an improvement.

@XmiliaH
Copy link
Collaborator

XmiliaH commented Feb 10, 2022

Should be fixed in v3.9.7

@XmiliaH XmiliaH closed this as completed Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants