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

allowing use of require() in tasks #154

Open
TomKaltz opened this issue Jul 5, 2016 · 3 comments
Open

allowing use of require() in tasks #154

TomKaltz opened this issue Jul 5, 2016 · 3 comments

Comments

@TomKaltz
Copy link

TomKaltz commented Jul 5, 2016

I would like to use require() in my task body. I have found a way to hack this capability by adding...
require: require

to https://github.com/joyent/node-workflow/blob/58715e9835d80156370035354143acb2add92c86/lib/task-runner.js#L53

This allows me to use require() relative to the task-runner.js file of the wf module. Including a script from my project root would be require('../../../thescript')

I had to modify the source of this module to get require to work properly because specifying require: require in the runner sandbox modules config doesn't work.

I understand that the use of vm is for security purposes but is there a safe way to allow the use of require in task body? Perhaps allow the user to specify require in runner config with a disclaimer that doing so would lower safety?

@kusor
Copy link
Contributor

kusor commented Jul 6, 2016

Honestly, I don't think it's a good idea to allow the use of require from tasks: what is the reason to do not require those modules in advance using wf-runner configuration?

Not to mention that we would be moving from "dangerous" to "completely unsafe" in terms of executing completely arbitrary code from vm.

@TomKaltz
Copy link
Author

TomKaltz commented Jul 6, 2016

I would like to be able to reuse code and reduce boilerplate in my tasks. As far as security, I don't understand how this is any less safe. Your database and file system already hold 'arbitrary' code that is run anyway. We just don't have the option to require arbitrary files for our arbitrary code. Not sure how sandboxing in this case makes anything any safer. I don't understand where malicious injection would occur otherwise.

@kusor
Copy link
Contributor

kusor commented Aug 4, 2016

@TomKaltz allowing a task to use require means that it could also require things like fs and, for example, fork a child process, which could mess pretty much everything regarding how wf-runner work.

Anyway, I wouldn't be opposed to add a configurable option to use require and have it disabled by default with that disclaimer you mentioned

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