Running shell commands in a safe manner

[johtso]

Am I not right in thinking that term and exec are both vulnerable to command injection attacks (or even just accidents) when user input is passed to commands?

Say for example the input contained something like ; rm -rf /;.

Should there perhaps be a warning in the documentation? A quick search and it looks like there are loads of places where exec is being used and variables are being added to command strings using template literals.

Is there not an equivalent to Python's subprocess.run which handles all that for you?
I think in child_process perhaps has methods that are less dangerous?

[johnlindquist]

Right below the download button is this warning:

The entire node API is available to import for anyone who wants to write a more "safe" script. This article has some suggestions around using execFile and sanitizing input/output.

https://auth0.com/blog/preventing-command-injection-attacks-in-node-js-apps/

[johtso]

Thanks for that! I'll take a read.

I guess an interesting question to ask is whether using exec to run commands is an "ok" pattern, whether it should be encouraged, and whether it would make sense to try and set a good example (where practical).

Having a general warning of "scripts are dangerous" is all well and good, but if there's a particular very common pattern that opens the tinkerer up to some potential nasty consequences, it might make sense to have a specific "here be dragons" warning.

I'm too green with node to have a solid opinion, I'm not really familiar with the available modules / apis for running commands, and only have a rudimentary awareness of these issues from the Python community where people are constantly warning "Don't just use os.system(), it's dangerous!".

Just thought it might be a worthwhile thing to bring up!

[johnlindquist]

As some examples: Script Kit bundles a very popular zx package which does the same as exec:

https://github.com/google/zx

There's not a single warning anywhere on their docs/issues/discussions.

---

iTerm, Hyper, etc don't have warnings that "running commands is dangerous".

---

I totally agree with you, "here be dragons". To me, it's kind of the territory of scripting/automation in general. I don't see why Script Kit should be held to a different standard of caution than other tools that do the same thing. Maybe I'm missing something?

[johtso]

Ah yeah, was just going to reply that I just found the example that uses zx which automatically escapes things. That seems like a fantastic balance of convenience and flexibility!

I'm not really sure where I got the idea that the examples were littered with exec usage with template literals. I guess the only place where maybe zx should be suggested instead of exec, or a little mention about needing to be careful with escaping would be here: https://github.com/johnlindquist/kit/blob/main/API.md#term

But yeah, I think it's all pretty ok.. just possibly worth while keeping in mind the user that is familiar with javascript, familiar with shell scripting, but is combining the two for the first time when they use ScriptKit. A little nudge/warning where exec usage is being suggest could be worthwhile, and wouldn't hurt. But I don't feel too strongly about it!

Appreciate your input and links, and am really excited to use this tool that you've built. It looks absolutely wonderful, and has made me really excited to automate "all the things".