search

moleculerjs / moleculer-repl

REPL module for Moleculer framework
http://moleculer.services/docs/moleculer-repl.html
MIT License
27 stars 25 forks source link

Let user call command with jsonParams as js syntax #38

Closed davidnussio closed 4 years ago

davidnussio commented 4 years ago

Example, support both syntaxt:

mol $ call math.add '{"a": 3, "b": 5}' mol $ call math.add '{a: 3, b: 5}'

icebob commented 4 years ago

second option is not a valid json, so JSON.parse throws error. Do you know package which can convert it into POJO?

davidnussio commented 4 years ago

I added jsonParser function in the utils.js file.

Function try to parse string with JSON.parse and if it fails use eval. I used safe-eval to introduce a minimum of security around eval. I know that call on repl is not expose to security attacks.

I added two tests. The eval fallback parse is very slow compared JSON.parse but is not a problem in repl call command.

intech commented 4 years ago

I am against this PR. convenience cannot be above safety. This solution does not limit the execution to json only, but makes it possible to execute any code in vm.

davidnussio commented 4 years ago

@intech about safety you are right but if someone with bad intentions has access to repl console the system security is over.

I used safe-eval that permit to use v8 api but should shield node api.

I prefer convenience over safety in development environment but in production is better switch mental mode to paranoid.

What do you think if "eval parse mode" is only active in repl started with flag? (--unsafe, --jsParamParase, --dev or something like this?

AndreMaz commented 4 years ago

I agree with @intech This is a security risk and I don't see huge benefit of supporting js syntax. JSON syntax works fine and it's safe. Also, I'm against the idea of accepting this with some special flag. People tend to forget things meaning that someone somewhere would forget to disable this flag and it would leave the entire system compromised.

davidnussio commented 4 years ago

Ok, I understood yours point of view.

I and my colleague get json parse exception because forget " around json properties name (we use too often chrome console) but in fact to simplify development phase the better solution could be implemented a moleculer vscode dashboard extension.

In this way development will get more benefits and no security flaws will be introduced into molecular-repl. 👍

icebob commented 4 years ago

Or you can use a custom REPL with nodejs repl package which same as Chrome devtools console: https://github.com/moleculerjs/moleculer/issues/252#issuecomment-386831330