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

Let developers to call JS methods on limited JS objects #580

Closed
pkarw opened this issue Apr 23, 2021 · 9 comments
Closed

Let developers to call JS methods on limited JS objects #580

pkarw opened this issue Apr 23, 2021 · 9 comments
Labels
enhancement New feature or request

Comments

@pkarw
Copy link
Collaborator

pkarw commented Apr 23, 2021

Is your feature request related to a problem? If yes, please describe the problem.

It would be great if we could pass over some libraries - take the lodash to AskQL and let the developers use all the functions provided, without the need to pack every call with a painstakingly created resource (current case). It could be pretty much useful for using the external services APIs like commercetools sdk or ORMs generated from tools like Prisma

To keep it safe we can let call just the top-level functions, or add whitelisting + only on the explicitly passed resources like:

const vmContext = { 
customResources:  [ 
 "lodash": jsModuleInterop(_, ["sort", "unique"])
]

where the jsModuleInterop is a wrapper over the JS object accepting the object (in this case _ - lodash module, plus a whitelist of methods)

@pkarw pkarw added the enhancement New feature or request label Apr 23, 2021
@czerwinskilukasz1
Copy link
Collaborator

Sounds good. In case of lodash probably there are no harmful functions, but in general it would be good to have jsModuleInterop.

@pkarw
Copy link
Collaborator Author

pkarw commented May 3, 2021

Cool! Any insights on how can we implement this feature? I was looking at the run method inside the vm implementation. Is it the right hook-up point to extend it? I mean we could potentially implement a "hack": to check the 1st parameter of any function call (as it should be the processing resource): if there is a proper child function - lets' call it. However, it's not an ideal one. I mean when these functions are going to return JS objects (chaining) we'd be not able to use it further.

@czerwinskilukasz1
Copy link
Collaborator

I envision jsModuleInterop the following way:

const vmContext = { 
  customResources: [ 
   'lodash': jsModuleInterop('lodash', ['sort', 'unique'])` 
  ]
...
}

What jsModuleInterop(arg1, args) would be is a helper that creates a resource which has properties listed in args that are wrappers over functions with the same names from module arg1.

@czerwinskilukasz1
Copy link
Collaborator

This way we don't need to add a hack with additional checks for function names in runtime. We use an existing mechanism instead.

@pkarw
Copy link
Collaborator Author

pkarw commented May 14, 2021

That's cool, however I've got an additional question. What do you mean by resource properties? I mean at the moment one resource could have just a single execute and secondly resolve method - so no additional properties could be set. Or am I wrong?

@pkarw
Copy link
Collaborator Author

pkarw commented May 14, 2021

@czerwinskilukasz1 maybe another option would be to add a support for something like JS scripting inside AskQL? I mean something like:

js { 
}

when used within the AskScript the JS interpreter like jerryScript or JS-interpreter is being used to execute the scope - isolated from the main Node process.

@lukasz-xfaang
Copy link

That's cool, however I've got an additional question. What do you mean by resource properties? I mean at the moment one resource could have just a single execute and secondly resolve method - so no additional properties could be set. Or am I wrong?

Actually, I should have written 'dictionary' rather than 'resource', so that lodash.sort in AskScript would be a function for sorting, lodash.unique would be a function for unique values etc.

@czerwinskilukasz1 maybe another option would be to add a support for something like JS scripting inside AskQL? I mean something like:

js { 
}

when used within the AskScript the JS interpreter like jerryScript or JS-interpreter is being used to execute the scope - isolated from the main Node process.

If we allow to run an arbitrary Javascript code, we would need to make sure that it cannot make nasty things on the server, e.g. access files, write/read files, open ports etc., e.g using os or net packages, which are available by default. Then, we would need to either whitelist only specific packages or block any require() / import at all. In any case, we would run in the whole class of issues we would run if we wanted to run a Javascript interpreter in the first place. Not sure if we want to focus on it now.

@mhagmajer
Copy link
Contributor

@pkarw seems like this can be done in user space with a resource factory that accepts an object, for example:

  function myResourceFactory(module) {
    const res = {}
    Object.keys(module).forEach((k) => {
      res[k] = resource({
        type: any,
        async resolver(...args: any) {
          return module[k](...args);
        },
      });
    });
    return res;
  }

  const result = await askql.runUntyped(
    {
      resources: {
        ...myResourceFactory(lodash),
        ...askql.resources,
      }
    },
    askql.parse("ask { 'hello world!' }")
  );

The design decision with AskQL was to have a separate computing environment from JavaScript (or any other host programming language) and therefore it would be troublesome to do a direct linking. All callable host methods would need to go through an explicit wrapper which allows for security measures. With the method above it perhaps becomes easier to add multiple resources at the same time.

Although, for full support of what you're asking for with namespaces would need to solve #579 first.

@pkarw
Copy link
Collaborator Author

pkarw commented May 18, 2021

Perfect fit @mhagmajer. Thanks!

pkarw added a commit that referenced this issue May 20, 2021
@pkarw pkarw closed this as completed Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants