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

TypeError: 'get' on proxy: property 'constants' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value #62

Closed
karthikv opened this issue Mar 6, 2017 · 15 comments

Comments

@karthikv
Copy link

karthikv commented Mar 6, 2017

Noticed this while trying to require a dependency in a vm. Some code to reproduce:

const { NodeVM } = require("vm2");

const vm = new NodeVM({
  console: "redirect",
  sandbox: {},
  require: {
    builtin: ["fs"]
  },
  wrapper: "none"
});

vm.run("const fs = require('fs'); fs.constants");

Full stack trace:

node_modules/vm2/lib/main.js:327
			throw this._internal.Decontextify.value(e);
			^
TypeError: 'get' on proxy: property 'constants' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value (expected '[object Object]' but got '[object Object]')
    at Object.<anonymous> (vm.js:1:91)
    at NodeVM.run (node_modules/vm2/lib/main.js:325:27)
    at Object.<anonymous> (script.js:12:4)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.runMain (module.js:590:10)
    at run (bootstrap_node.js:394:7)

Let me know if there's any other information I can give you to be helpful!

@kellym
Copy link

kellym commented Mar 14, 2017

I'm also having quite a few issues around proxies, specifically with inheritance. I'm not sure if these issues are due to the implementation or a limitation of Proxies, but it's worth some investigation.

@mjbvz
Copy link

mjbvz commented Jun 11, 2017

Also ran into this error. I'm not familiar enough with proxies to understand why the current pattern is not ok but I was able to make the error go away by adding an extra check for readonly non-configurable properties in Contextify.object.get:

	object: function(object, traps, deepTraps, flags, mock) {
		const proxy = new host.Proxy(object, host.Object.assign({
			get: (target, key, receiver) => {
				if (key === 'vmProxyTarget' && DEBUG) return object;
				if (key === 'isVMProxy') return true;
				if (mock && key in mock) return mock[key];
				if (key === 'constructor') return Object;
				if (key === '__proto__') return Object.prototype;
                               
                                // Added code
				const config = host.Object.getOwnPropertyDescriptor(object, key);
				if (config && config.configurable === false && config.writable === false) {
					return object[key];
				}

This seems to follow the get invariant MDN describes:

The value reported for a property must be the same as the value of the corresponding target object property if the target object property is a non-writable, non-configurable data property.

But it seems to me like it would break the sandboxing

@terryrgrs
Copy link

Just chiming in to note that we've run into this issue locally as well, with the above code change allowing us to bypass the problem. Also noting that this happened while using the Google Sheets node module in an almost exact copy of their quickstart guide (https://developers.google.com/sheets/api/quickstart/nodejs).

Are there any updates on this issue?

@ghost
Copy link

ghost commented Oct 26, 2017

Does this fix have an effect on the known issue "It is not possible to define class that extends proxied class?" What do you mean it would break the sandboxing?

@Madd0g
Copy link

Madd0g commented Jun 14, 2018

Hey, any progress on this issue?
I'm tried to use libraries like graceful-fs and got the same error.

@fnjoe
Copy link

fnjoe commented Sep 21, 2018

I met this error when i exported a ES6 class in the script used to run. it was ok after i translated it into
ES5.

@patriksimek
Copy link
Owner

Unfortunately, the solution posted by @mjbvz breaks the sandbox. I don't think there is a way to fix that issue - it the Proxy for read-only object can't return anything else except the original value, then we're screwed and throwing the error is the only way to keep sandbox secure.

Ideas are very welcome.

@eight04
Copy link

eight04 commented Jan 27, 2019

Can we get a clone in this situation?

@patriksimek
Copy link
Owner

Clone doesn't help - a clone is also a distinct value.

@eight04
Copy link

eight04 commented Jan 28, 2019

I meant a clone of fs.constants, so it is no longer read-only.

@patriksimek
Copy link
Owner

That would allow a sandbox to make changes to the object which could cause problems within the host's scope. You need to be very careful about what you send to the sandbox because every change to the object made by the sandbox is also effective in the host's context.

@ryzokuken
Copy link

If it's read-only, do you even need a proxy to verify stuff?

@digvan
Copy link

digvan commented Mar 18, 2020

Is there any update on this issue?
I am getting same issue when using googleapi node module ...

@XmiliaH
Copy link
Collaborator

XmiliaH commented Mar 21, 2020

Fixed with #252.

@XmiliaH XmiliaH closed this as completed Mar 21, 2020
@digvan
Copy link

digvan commented Mar 22, 2020

@XmiliaH Thank you! really appreciate it. It is working with latest version.

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