patriksimek / vm2

Advanced vm/sandbox for Node.js
MIT License
3.86k stars 295 forks source link

Modules not loading any more? #520

Closed fcbrandon closed 1 year ago

fcbrandon commented 1 year ago

Modules loaded via require don't seem to work anymore, I always get a VMError: Cannot find module <module>.

Even when running one of the examples from the README, I get the following: VMError: Cannot find module 'request'

I had been using vm2 for quite a while with no problem, but recently came back to the project using it, and now nothing seems to work.

Should I be running an older version of Node, or something else, for vm2 to work correctly again?

XmiliaH commented 1 year ago

What options do you give to NodeVM and how do you run the module?

fcbrandon commented 1 year ago

I tried running this example from the README and am having the same issue:

const {NodeVM} = require('vm2');
const vm = new NodeVM({
    require: {
        external: true,
        root: './'
    }
});

vm.run(`
    var request = require('request');
    request('http://www.google.com', function (error, response, body) {
        console.error(error);
        if (!error && response.statusCode == 200) {
            console.log(body); // Show the HTML for the Google homepage.
        }
    });
`, 'vm.js');
XmiliaH commented 1 year ago

Did you install the request module with npm install request?

fcbrandon commented 1 year ago

I did run npm install request, and I can see it in the node_modules.

For the heck of it, I pulled the code to run into its own file and that ran with no problem.

So I guess loading modules should be working fine, like, there hasn't been a change that broke module loading?

XmiliaH commented 1 year ago

There should not be a change that broke module loading. The example worked fine for me. I created a test.js file with the example code and ran:

npm install vm2
npm install request
node test.js

and I got a resopnse back and printed.

fcbrandon commented 1 year ago

So then it's something local on my machine?

Here's exactly what I'm seeing:

<redacted>/node_modules/vm2/lib/bridge.js:487
                throw thisFromOtherForThrow(e);
VMError: Cannot find module 'request'
    at DefaultResolver.resolveFull (<redacted>/node_modules/vm2/lib/resolver.js:108:9)
    at DefaultResolver.resolveFull (<redacted>/node_modules/vm2/lib/resolver.js:315:16)
    at DefaultResolver.resolve (<redacted>/node_modules/vm2/lib/resolver.js:103:15)
    at ReadOnlyHandler.apply (<redacted>/node_modules/vm2/lib/bridge.js:485:11)
    at requireImpl (<redacted>vm2/lib/setup-node-sandbox.js:84:28)
    at require (/<redacted>/vm2/lib/setup-node-sandbox.js:165:10)
    at vm.js:2:19
    at VM2 Wrapper.apply (<redacted>/node_modules/vm2/lib/bridge.js:485:11)
    at NodeVM.run (<redacted>/node_modules/vm2/lib/nodevm.js:426:23)
    at Object.<anonymous> (<redacted>/temp/index.js:9:4) {
  code: 'ENOTFOUND'
}
XmiliaH commented 1 year ago

I am not able to reproduce your issue, but there should be nothing machine depndent there. The file structure should be like:

./test.js
./node_modules/request/...

So in your case you should find the request module in <redacted>/temp/node_modules/request/....

fcbrandon commented 1 year ago

The request module is definitely there. Again, I can run the 'request' code in its own file, it runs just fine, so it's something specific to vm2.

I just tried the sample in a new Ubuntu VM and it ran just fine (I totally trusted your response, more just to confirm I could get things to run for myself as well.). So there's definitely something with my machine. It's an M1 Macbook Pro; I doubt that has anything to do with it though.

I'll try to uninstall Node/NPM and try it again, hopefully there's just something up with my Node install. I'll report back with status.

So far this is all good news for me; I was starting to get pretty bummed that I'd have to find a new isolation library, which would have been a pain 😆

XmiliaH commented 1 year ago

I do not have a M1 Macbook Pro to test. You could set a breakpoint in the function resolveFull in file lib/resolver.js at line 225 and step through the resolve process. The module should be loaded through resolveFull > loadNodeModules > loadAsDirectory > loadAsPackage.

fcbrandon commented 1 year ago

There's something up with how the paths are detected, I'm still working on figuring it out though. The difference it notable at this line: https://github.com/patriksimek/vm2/blob/24c724daa7c09f003e556d7cd1c7a8381cb985d7/lib/resolver.js#L309

        // 6. LOAD_NODE_MODULES(X, dirname(Y))
        f = this.loadNodeModules(x, dirs, extList);
        if (f) return f;

When running on my Macbook, dirs has a length of zero, but in Ubuntu, it looks something like this:

[ 
0: '/home/source/project/node_modules',
1: '/home/source/node_modules',
2: '/home/node_modules'
3: '/node_modules'
]

It's not immediately clear why it's happening for me, but it's clear that on the Macbook vm2 is unable to enumerate paths for node_modules, which is why it keeps failing.

XmiliaH commented 1 year ago

That seems to be the problem. The list is generated in genLookupPaths but I do not see why this should not work with the Macbook.

fcbrandon commented 1 year ago

Just uninstalled Node and used nvm to reinstall the current LTS - got the exact same issue. 🤔

I'll see if I can debug what's up with genLookupPaths then.

fcbrandon commented 1 year ago

Debugging found that options passsed into resolveFull is undefined, which won't it through the check on line 282: https://github.com/patriksimek/vm2/blob/24c724daa7c09f003e556d7cd1c7a8381cb985d7/lib/resolver.js#L282 So the code is never hitting genLookupPaths.

options is set as such over in: https://github.com/patriksimek/vm2/blob/24c724daa7c09f003e556d7cd1c7a8381cb985d7/lib/setup-node-sandbox.js#L84

On my working Ubuntu instance, the code never hits genLookupPaths either though. On my build, paths come from mod.paths.

So I traced that back, and the paths seem to be based on the options variable passed in at: https://github.com/patriksimek/vm2/blob/24c724daa7c09f003e556d7cd1c7a8381cb985d7/lib/nodevm.js#L369

And this is the first place I'm seeing the difference between my two instances. On the working Ubuntu instance, options is set to 'vm.js'.

image

However, on my Macbook, it's undefined:

image

Sooooo, I went back into the original example code, and manually added an option as 'vm.js':

vm.run(`
    var request = require('request');
    request('http://www.google.com', function (error, response, body) {
        console.error(error);
        if (!error && response.statusCode == 200) {
            console.log(body); // Show the HTML for the Google homepage.
        }
    });`, "vm.js");

And now everything works fine. I checked the version of Node and NPM on each instance, but they're exactly the same - Node: v18.16.0, NPM: 9.5.1. I have zero idea why this happens on the Macbook, and not in Ubuntu, but now I can see what is happening, and have a pretty simple work around.

If anyone wants me to try further debugging, send me some instructions and I'll see if I can help figure out what's happening here.

Thanks for brainstorming this with me @XmiliaH - it helps to have someone else to bounce these things off and work through out!!!

XmiliaH commented 1 year ago

Sooooo, I went back into the original example code, and manually added an option as 'vm.js':

This is required and is set in the examples. Without it only absolute modules can be loaded.

fcbrandon commented 1 year ago

Nonetheless, it works in some scenarios, and not in others.

And if it's required, then I guess we should update README, accordingly?

XmiliaH commented 1 year ago

Nonetheless, it works in some scenarios, and not in others.

Yes, there are scenarios where it should not work as one can restrict the lookup paths.

And if it's required, then I guess we should update README, accordingly?

Where should it be updated? The example posted in https://github.com/patriksimek/vm2/issues/520#issuecomment-1507516488 has it set.