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

How to pass JSON to VM? I get "A non-transferable value was passed" #514

Closed
caub opened this issue Dec 20, 2024 · 9 comments
Closed

How to pass JSON to VM? I get "A non-transferable value was passed" #514

caub opened this issue Dec 20, 2024 · 9 comments

Comments

@caub
Copy link

caub commented Dec 20, 2024

It seems that whenever passing an object as context value it fails

import ivm from "isolated-vm";
const isolate = new ivm.Isolate();
const context = await isolate.createContext();
context.global.setSync('JSONParse', JSON.parse) // this works
context.global.setSync('JSONStringify', JSON.stringify) // this works

context.global.setSync('JSON', Object.assign(Object.create(null), { parse: JSON.parse, stringify: JSON.stringify }))
// TypeError: A non-transferable value was passed
@laverdet
Copy link
Owner

Take a look at TransferOptions in the docs

@caub
Copy link
Author

caub commented Dec 20, 2024

Sorry I don't manage to get it yet

const ivm = require('isolated-vm');
const isolate = new ivm.Isolate({ memoryLimit: 16 });
const context = isolate.createContextSync();

// this works
context.global.setSync('JSONs', v=>JSON.stringify(v), {})
context.evalSync('JSONs(null)')
// 'null'

// this doesn't work yet, I'm probably misunderstanding
context.global.setSync('JSON', {s: v=>JSON.stringify(v)}, {copy:true}) // trying this option
// Uncaught TypeError: v=>JSON.stringify(v) could not be cloned.
context.global.setSync('JSON', {s:v=>JSON.stringify(v)}, {reference:true}) // trying this other option
// It doesn't throw but
context.evalSync('JSON.s(null)')
// Uncaught TypeError: JSON.s is not a function

Optional question (if you don't mind) are there still benefits in passing require('secure-json-parse').parse instead of JSON.parse for example in the isolate?

@laverdet
Copy link
Owner

JSON is included in v8, you don't need to build it yourself.

Otherwise, you are trying to pass a function to an isolate. It's not how isolates work. This is covered in the frequently asked question: https://github.com/laverdet/isolated-vm?tab=readme-ov-file#frequently-asked-question

@caub
Copy link
Author

caub commented Jan 6, 2025

We're trying to migrate out of vm2 like plenty people here, I've seen in the issues, but it's challenging to use this lib on this part: "you will need to set up some kind of shim delegate which can perform the operation within nodejs and pass the result back to your isolate", because we have a add-on framework, allowing to run code, with one special argument: api (which has DB side-effects)

Here's a small example of such code:

async function onUserUpdated({ api, data }) {
  const user = await api.get(`users/${data.userId}`);
  if (user.signinAt < Date.now() - 86400000) {
    await api.put(`users/${data.userId}`, { isActive: false });
    return { note: 'updated' };
  }
}

I've been thinking quite a lot on how to proxy data with the isolate, but since there can be any number of api calls, it's difficult

Maybe the best idea I have is to ensure this code is formatted a bit like React functional components with hooks, that way we would be able to fetch all data with api.get() using these hooks at the beginning of the function, then run the code, and use returned values to know if we have to make api.put() calls. But it's not that practical for us

The other solution for us is to use Node's vm to ensure the code doesn't run too long and doesn't use too much memory, and for the security part we'd use eslint to ensure there are no side-effects except using api.put()

If you've time later, not urgent, I'd be very gratful to hear your thoughts

Thanks for your work on this clever lib

@laverdet
Copy link
Owner

laverdet commented Jan 6, 2025

node vm is made for unit tests, not for security. vm2 is just a thing on top of vm. Literally, just think of isolated-vm as a web browser. How would you write this if isolated-vm was a browser and you wanted to run the database calls on the server?

@caub
Copy link
Author

caub commented Jan 7, 2025

In a browser / server scenario, I'd use fetch, it's not available in ivm obviously, but I'd love to be able to transfer a sort of black box like fetch that we trust, not sure it's possible, even using ivm.Reference?

Otherwise I think we need to communicate back and forth between node and isolate, but it's not very easy in our case as we'd like to run one chunk of code in one go

I think my idea 1 in previous message, similar to hooks could work, but it's restrictive for us, and we have to rewrite them to this type of format

async function onUserUpdated({ data }) {
  const user = get(`users/${data.userId}`);
  if (user.signinAt < Date.now() - 86400000) {
    return [{ method: 'put', path: `users/${user.id}`, body: { isActive: false } }];
  }
}

We could parse this code as an AST and extract all get() calls, run them in node and replace them with their value

async function onUserUpdated({ data }) {
  const user = { id: 'userId1', email: '....' };
  if (user.signinAt < Date.now() - 86400000) {
    return [{ method: 'put', path: `users/${user.id}`, body: { isActive: false } }];
  }
}

We can run this in ivm and then use the result to execute the final api mutations

@caub
Copy link
Author

caub commented Jan 8, 2025

With #322 and #49 I"m able to get something close

import ivm from 'isolated-vm';
const isolate = new ivm.Isolate({ memoryLimit: 16 });
const context = await isolate.createContext();

const global = context.global;

await global.set('log', console.log);
await global.set('api', new ivm.ExternalCopy({}).copyInto());
const api = await global.get('api')
await api.set('get', new ivm.Reference(
  async function (path) {
    const r = await fetch(`https://dummyjson.com${path}`);
    return r.json();
  }
));

const fn = await context.eval(`
  (async function onUserUpdated() {
    const user = await api.get.applySyncPromise(null, ['/users/1'], { result: { copy: true } });
    log('fetched user', user.id);
    return user;
  })`, { reference: true });

const value = await fn.apply(undefined, [], { result: { promise: true, copy: true } })
console.log(value);

But I get an error: (according to the docs that's a valid option)

TypeError: `result` options are not available for `applySyncPromise`
    at onUserUpdated (<isolated-vm>:3:32)

With

const user = await api.get.applySyncPromise(null, ['/users/1'], { copy: true });

I get

TypeError: A non-transferable value was passed
    at onUserUpdated (<isolated-vm>:3:32

If using return r.text() instead of return r.json() things work, but we need to manipulate objects inside the isolate, as soon as we JSON.parse it the error happens

Another issue is I'd like to pass the userId as an object argument ['users/' + data.userId] but we can't pass both copy: true and reference: true options to eval()

@laverdet
Copy link
Owner

laverdet commented Jan 9, 2025

Objects aren't transferable. You would need to use ExternalCopy / copyInto. I don't think you need applySyncPromise for this, you are making your life harder than it needs to be. That API is meant for replicating fs.readSync without blocking the main nodejs loop.

@caub
Copy link
Author

caub commented Jan 9, 2025

Ok thanks, here it is, it may work for our usecases, thanks

import ivm from 'isolated-vm';
const isolate = new ivm.Isolate({ memoryLimit: 16 });
const context = await isolate.createContext();
const global = context.global;
await global.set('log', console.log);
await global.set('fetchDummmy', new ivm.Reference(
  async function (path) {
    const r = await fetch(`https://dummyjson.com${path}`);
    const o = await r.json();
    console.log('node fetchDummmy:', o.id);
    return new ivm.ExternalCopy(o);
  }
));
const fn = await context.eval(`
(async function onUserUpdated() {
  const user = await fetchDummmy.applySyncPromise(null, ['/users/1']);
  log('isolate:', user.copy());
  return user;
})`, { reference: true });

const value = await fn.apply(null, [], { result: { promise: true, copy: true } })
console.log('node result:', value);
node fetchDummmy: 1
isolate: {
  id: 1,
  firstName: 'Emily',
}
node result: {
  id: 1,
  firstName: 'Emily',
}

I didn't manage to have it working without applySyncPromise

Tried something like

  const userRef = await fetchDummmy.apply(null, ['/users/1'], { promise: true });
  const user = await userRef.derefInto();
  log('isolate:', user);

but user is still a Promise there, and 'node result:' runs before 'node fetchDummy:', it should not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants