Search code examples
pythonshellpopen

How to avoid passing shell constructs to executable using Popen


I am trying to call an executable called foo, and pass it some command line arguments. An external script calls into the executable and uses the following command:

./main/foo --config config_file 2>&1 | /usr/bin/tee temp.log

The script uses Popen to execute this command as follows:

from subprocess import Popen
from subprocess import PIPE
def run_command(command, returnObject=False):
    cmd = command.split(' ')
    print('%s' % cmd)
    p = None
    print('command : %s' % command)
    if returnObject:
        p = Popen(cmd)
    else:
        p = Popen(cmd)
        p.communicate()
        print('returncode: %s' % p.returncode)
        return p.returncode
    return p

command = "./main/foo --config config_file 2>&1 | /usr/bin/tee temp.log
"
run_command(command)

However, this passes extra arguments ['2>&1', '|', '/usr/bin/tee', 'temp.log'] to the foo executable.

How can I get rid of these extra arguments getting passed to foo while maintaining the functionality? I have tried shell=True but read about avoiding it for security purposes (shell injection attack). Looking for a neat solution.

Thanks

UPDATE: - Updated the file following the tee command


Solution

  • The string

    ./main/foo --config config_file 2>&1 | /usr/bin/tee >temp.log
    

    ...is full of shell constructs. These have no meaning to anything without a shell in play. Thus, you have two options:

    • Set shell=True
    • Replace them with native Python code.

    For instance, 2>&1 is the same thing as passing stderr=subprocess.STDOUT to Popen, and your tee -- since its output is redirected and it's passed no arguments -- could just be replaced with stdout=open('temp.log', 'w').


    Thus:

    p = subprocess.Popen(['./main/foo', '--config', 'config_file'],
      stderr=subprocess.STDOUT,
      stdout=open('temp.log', 'w'))
    

    ...or, if you really did want the tee command, but were just using it incorrectly (that is, if you wanted tee temp.log, not tee >temp.log):

    p1 = subprocess.Popen(['./main/foo', '--config', 'config_file'],
      stderr=subprocess.STDOUT,
      stdout=subprocess.PIPE)
    p2 = subprocess.Popen(['tee', 'temp.log'], stdin=p1.stdout)
    p1.stdout.close() # drop our own handle so p2's stdin is the only handle on p1.stdout
    stdout, _ = p2.communicate()
    

    Wrapping this in a function, and checking success for both ends might look like:

    def run():
        p1 = subprocess.Popen(['./main/foo', '--config', 'config_file'],
          stderr=subprocess.STDOUT,
          stdout=subprocess.PIPE)
        p2 = subprocess.Popen(['tee', 'temp.log'], stdin=p1.stdout)
        p1.stdout.close() # drop our own handle so p2's stdin is the only handle on p1.stdout
        # True if both processes were successful, False otherwise
        return (p2.wait() == 0 && p1.wait() == 0)
    

    By the way -- if you want to use shell=True and return the exit status of foo, rather than tee, things get a bit more interesting. Consider the following:

    p = subprocess.Popen(['bash', '-c', 'set -o pipefail; ' + command_str])
    

    ...the pipefail bash extension will force the shell to exit with the status of the first pipeline component to fail (and 0 if no components fail), rather than using only the exit status of the final component.