[Python-ideas] shutil.runret and shutil.runout

[Masklinn]

On 2012-02-24, at 14:00 , anatoly techtonik wrote:
> On Fri, Feb 24, 2012 at 2:50 PM, Masklinn <masklinn at masklinn.net> wrote:
>> On 2012-02-24, at 12:12 , anatoly techtonik wrote:
>>> 
>>> 1. they require try/catch
>> 
>> No.
> 
> Quote from the docs:
> "Run command with arguments. Wait for command to complete. If the
> return code was zero then return, otherwise raise CalledProcessError."
> http://docs.python.org/library/subprocess.html#subprocess.check_call

Yes. If you want to run commands you just do. try/except are only needed
if you call commands which may fail and want to handle them without
quitting the whole interpreter. And for your stated use case of
interactively calling those functions, there is no need whatsoever for
try/catch.

And `subprocess.call` returns the status code, no exception ever thrown.

>>> 3. contain shell FUD
>> 
>> No, they contain warnings, against shell injection security
>> risks. Warnings are not FUD, it's not trying to sell some sort
>> of alternative it's just warning that `shell=True` is dangerous
>> on untrusted input.
> 
> Warnings would be o.k. if they provided at least some guidelines where
> shell=True can be useful and where do you need to use Popen (or
> escaping). Without positive examples, and a little research to show
> attack vectors (so that users can analyse if they are applicable in
> their specific case) it is FUD IMO.

http://docs.python.org/library/subprocess.html#frequently-used-arguments

>>> 4. completely confuse users with stdout=PIPE or stderr=PIPE stuff
>>> 
>>> http://docs.python.org/library/subprocess.html#subprocess.check_call
>> 
>> On the one hand, these notes are a bit clumsy. On the other hand,
>> piping is a pretty fundamental concept of shell execution, I see
>> nothing wrong about saying that these functions *can't* be involved
>> in pipes. In fact stating it upfront looks sensible.
> 
> The point is that it makes things more complicated than necessary.

How?

> As
> a system programmer I feel confident about all this stuff

You feel confident about something which does not work, without warning?

>>>> If you do "pip install shell-command" you can also access the
>>>> shell_call(), shell_check_call() and shell_output() functions I
>>>> currently plan to include in subprocess for 3.3. (I'm not sure which
>>>> versions of Python that module currently supports though - 2.7 and
>>>> 3.2, IIRC).
>>> 
>>> Don't you find strange that shell utils module don't have any
>>> functions for the main shell function - command execution?
>> 
>> What "shell utils" module? Subprocess has exactly that in `call`
>> and its variants. And "shutil" does not bill itself as a
>> "shell utils" module right now, its description is
>> "High-level file operations".
>> 
>>> shutil.runret()  - by definition has shell=True
>> 
>> Great, so your recommendation is to be completely insecure by default?
> 
> Not "by default"

Oh? Because this:

> - only if it is impossible to make shutil.run*()
> functions more secure. They only make sense with shell=True, so my
> recommendation is to analyse security implications and *let* users
> make their grounded choice. Not frighten them, but making them think
> about security.
> 
> The difference. User friendly docs for shutil.run*() docs should be
> structured as following:
> 1. you are free to use these functions
> 2. but know that they are insecure
> 3. in these cases:
> 3.1
> 3.2
> 3.3
> 4. if you think these cases won't apply to your project, then feel
> free to use, otherwise look at subprocess
> 
> Of course, if some cases 3.1-3.3 have workarounds, they should be mentioned.

states precisely that the function would be insecure by default, and would
have caveat warnings in the docs. Which is the correct approach to
security… never as far as I know.

>>> The main purpose of this function is to be useful from Python console
>> 
>> Then I'm not sure it belongs in subprocess or shutil, and users with that
>> need should probably be driven towards iPython which provides extensive
>> means of calling into the system shell in interactive sessions[0].
>> bpython may also provide such facilities.
> 
> I think it is a good idea to unify interface across interactive mode
> in Python.

Considering IPython uses syntactic extentions (a "!" prefix)  and does not
require any importing effort currently, I doubt that's going to happen.

>>> It won't be 'shell util' function anymore. If you're using shell
>>> execution functions, you already realize that will happen if your
>>> input parameters are not validated properly.
>> 
>> This assertion demonstrably does not match reality, shell injections
>> (the very reason for this warning) would not exist if this were the
>> case.
> 
> It is not assertion,

You may want to look up the definition of that word, I did not remove any
context, you asserted people using shell-exec functions are aware of the
risks. Which is, as, factually wrong.

> it is a wannabe for shutil documentation to
> clarify shell injections problems to the level that allow users to
> make a reasonable choice, so if the user is "using shell execution
> functions he already realizes that will happen if his input parameters
> are not validated properly".

Not sufficient when the default behavior is unsafe (and broken), as
numerous users *will* discover the function through third parties and
may never come close to the caveats they *should* know for the default
usage of the function.