pyinvoke / invoke

Pythonic task management & command execution.
http://pyinvoke.org
BSD 2-Clause "Simplified" License
4.31k stars 365 forks source link

context.run(): Expose API to pass command and separate args instead of a single string #977

Open ibc opened 7 months ago

ibc commented 7 months ago

Currently context.run(command: str, **kwargs: Any) gives very limited chances to the user when dealing with paths with whitespaces or special chars.

mkdir "123\ foo\ \"bar"

Imagine that folder whose name contains whitespaces and double quote symbol. Passing it as argument to a shell command within a interpolated string is basically hell.

In Rust and Node you can spawn a separate command by specifying the command itself and then each argument separately (into an array of strings). This is very convenient to NOT having to deal with string interpolation. Example:

child_process.spawnSync(command, [ args ], ...);

However in Invoke all command arguments must be interpolated into a single command string. I don't know if this is a Python limitation. I just hope that Python exposes an API to specify the command and each separate argument to spawn. And if so, IMHO it would make sense that Invoke exposes it.

lieryan commented 6 months ago

Python's subprocesses.Popen() (and all the other convenience methods in the standard library that internally uses it as well as the low-level function calls to exec/spawn processes in os) in the standard library all do have support for spawning subprocesses using a list and in fact the list argument is the recommended way to spawn subprocesses, if you have to spawn subprocesses. The list argument is also necessary to call a multi-call binary (e.g. busybox) which is basically impossible to use in invoke. Using a shell to spawn subprocesses is a major security risk (and also it's bad for performance) and context.run() basically forces you to write insecure tasks.

Many security scanners such as bandit would flag use of subprocesses with the shell/string argument as high security risk. Using context.run() somewhat bypasses the scanners that don't recognise the invoke library, but all of the security risk is pretty much still there, and in fact, worse, since there isn't any way to actually do anything safely with context.run().

I'm actually rather surprised by the complete lack of consideration to do any sort of secure coding in invoke.run(). While there are many use case of invoke where shell security may not be important, there is not even any way for people who need to call subprocesses with semi/untrusted data using context.run().

achekery commented 3 months ago

Submitted https://github.com/pyinvoke/invoke/pull/987 for this qol change.