HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonModerate

Send commands over SSH to server

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
commandsserversendoverssh

Problem

Here's a very small and basic script that sends commands over SSH to another computer on my network:

#!/bin/python
import sys, os
commands = ""
for i in sys.argv[1:]:
        commands += " " + i;

if len(sys.argv) <= 1:
        os.system("sshpass -p VerySecrectPassword ssh pi@172.16.0.141")
else:
        os.system("sshpass -p VerySecrectPassword ssh pi@172.16.0.141" + commands)


Am I being very "Pythonic"? Is it better to use a for loop to join all the commands or the join function?

commands = " ".join(sys.argv[1:])


Personally, I think the for loop reads better, but that's just my opinion and I'm a newbie. Which is considered the better way to do things?

Furthermore:

  • Are there any other areas in this script that could be improved?



  • Is it a bad idea to store the password in the script (located at /usr/bin)?



Note: This script is stored on a computer where I am the only user (except root).

Solution

To answer your questions: Yes.

Your code could be boiled down to:

#!/bin/python
import sys
import os

os.system("sshpass -p VerySecrectPassword ssh pi@172.16.0.141" + " " .join(sys.argv[1:]))


Note that I also put the imports on two lines, as recommended by PEP8, Python's official style-guide.

But I would actually re-expand it to:

#!/bin/python
import sys
import os

data = {"user": "pi",
        "host": "172.16.0.141",
        "password": "VerySecret",
        "commands": " " .join(sys.argv[1:])}

command = "sshpass -p {password} ssh {user}@{host} {commands}"
os.system(command.format(**data))


This gives you some easier ways to modify the configuration. It would also be easier to implement reading this configuration from a file now.

Note that bash (or whatever shell you use) will not care about the trailing whitespace in case commands is the empty string.

The next step would be using a ssh library, like @Simon recommended in his comment.

One way to do it would be first setting up password-less login and then using this answer and a comment within it:

#!/bin/python
import sys
import paramiko

rpi = {"username": "pi",
       "hostname": "172.16.0.141"}
command = " " .join(sys.argv[1:])}

ssh = paramiko.SSHClient()
ssh.load_system_host_keys()
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
ssh.connect(**rpi)
ssh_stdin, ssh_stdout, ssh_stderr = ssh.exec_command(command)


Here the ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) is needed because paramiko does not read the hosts file correctly.

Code Snippets

#!/bin/python
import sys
import os

os.system("sshpass -p VerySecrectPassword ssh pi@172.16.0.141" + " " .join(sys.argv[1:]))
#!/bin/python
import sys
import os

data = {"user": "pi",
        "host": "172.16.0.141",
        "password": "VerySecret",
        "commands": " " .join(sys.argv[1:])}

command = "sshpass -p {password} ssh {user}@{host} {commands}"
os.system(command.format(**data))
#!/bin/python
import sys
import paramiko

rpi = {"username": "pi",
       "hostname": "172.16.0.141"}
command = " " .join(sys.argv[1:])}

ssh = paramiko.SSHClient()
ssh.load_system_host_keys()
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
ssh.connect(**rpi)
ssh_stdin, ssh_stdout, ssh_stderr = ssh.exec_command(command)

Context

StackExchange Code Review Q#143625, answer score: 10

Revisions (0)

No revisions yet.