Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pass parameters for shell function call using an array rather than (unquoted) string substitution #1608

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Pass parameters for shell function call using an array rather than (unquoted) string substitution #1608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions modoboa/admin/app_settings.py
View file
Open in desktop
Expand Up @@ -163,7 +163,7 @@ def __init__(self, *args, **kwargs):
}
hide_fields = False
dpath = None
code, output = exec_cmd("which dovecot")
code, output = exec_cmd(("which", "dovecot"))
if not code:
dpath = force_text(output).strip()
else:
Expand All @@ -176,7 +176,7 @@ def __init__(self, *args, **kwargs):
dpath = fpath
if dpath:
try:
code, version = exec_cmd("%s --version" % dpath)
code, version = exec_cmd((dpath, "--version"))
except OSError:
hide_fields = True
else:
Expand Down Expand Up @@ -213,7 +213,7 @@ def clean_dkim_keys_storage_dir(self):
raise forms.ValidationError(
ugettext_lazy("Directory not found.")
)
code, output = exec_cmd("which openssl")
code, output = exec_cmd(("which", "openssl"))
if code:
raise forms.ValidationError(
ugettext_lazy(
Expand Down
4 changes: 2 additions & 2 deletions modoboa/admin/management/commands/handle_mailbox_operations.py
View file
Open in desktop
Expand Up @@ -50,7 +50,7 @@ def rename_mailbox(self, operation):
except os.error as e:
raise OperationError(str(e))
code, output = exec_cmd(
"mv %s %s" % (operation.argument, new_mail_home)
("mv", operation.argument, new_mail_home)
)
if code:
raise OperationError(output)
Expand All @@ -77,7 +77,7 @@ def check_pidfile(self, path):
with open(path) as fp:
pid = fp.read().strip()
code, output = exec_cmd(
"grep handle_mailbox_operations /proc/%s/cmdline" % pid
("grep", "handle_mailbox_operations", "/proc/%s/cmdline" % pid)
)
if not code:
return False
Expand Down
4 changes: 2 additions & 2 deletions modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py
View file
Open in desktop
Expand Up @@ -27,13 +27,13 @@ def create_new_dkim_key(self, domain):
domain.dkim_key_length if domain.dkim_key_length
else self.default_key_length)
code, output = sysutils.exec_cmd(
"openssl genrsa -out {} {}".format(pkey_path, key_size))
("openssl", "genrsa", "-out", pkey_path, str(key_size)))
if code:
print("Failed to generate DKIM private key for domain {}: {}"
.format(domain.name, smart_text(output)))
domain.dkim_private_key_path = pkey_path
code, output = sysutils.exec_cmd(
"openssl rsa -in {} -pubout".format(pkey_path))
("openssl", "rsa", "-in", pkey_path, "-pubout"))
if code:
print("Failed to generate DKIM public key for domain {}: {}"
.format(domain.name, smart_text(output)))
Expand Down
7 changes: 2 additions & 5 deletions modoboa/core/commands/deploy.py
View file
Open in desktop
Expand Up @@ -220,11 +220,8 @@ def handle(self, parsed_args):
extensions = [(extension, extension.replace("-", "_"))
for extension in extensions]
if not parsed_args.dont_install_extensions:
cmd = (
sys.executable +
" -m pip install " +
" ".join([extension[0] for extension in extensions])
)
cmd = [sys.executable, "-m", "pip", "install"]
cmd.extend((extension[0] for extension in extensions))
exec_cmd(cmd, capture_output=False)
extra_settings = self.find_extra_settings(extensions)
extensions = [extension[1] for extension in extensions]
Expand Down
12 changes: 6 additions & 6 deletions modoboa/lib/sysutils.py
View file
Open in desktop
Expand Up @@ -22,16 +22,16 @@ def exec_cmd(cmd, sudo_user=None, pinput=None, capture_output=True, **kwargs):
Run a command using the current user. Set :keyword:`sudo_user` if
you need different privileges.

:param str cmd: the command to execute
:param collections.abc.Iterable cmd: the command to execute
:param str sudo_user: a valid system username
:param str pinput: data to send to process's stdin
:param bool capture_output: capture process output or not
:rtype: tuple
:return: return code, command output
"""
if sudo_user is not None:
cmd = "sudo -u %s %s" % (sudo_user, cmd)
kwargs["shell"] = True
cmd = ["sudo", "-u", sudo_user] + list(cmd)
kwargs["shell"] = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a good idea to change this value since you did not modify commands to include absolute pathes. If you disable shell mode, PATH resolution won't be available, right?

if pinput is not None:
kwargs["stdin"] = subprocess.PIPE
if capture_output:
Expand All @@ -53,15 +53,15 @@ def doveadm_cmd(params, sudo_user=None, pinput=None,
Run doveadm command using the current user. Set :keyword:`sudo_user` if
you need different privileges.

:param str params: the parameters to give to doveadm
:param collections.abs.Iterable params: the parameters to pass to doveadm
:param str sudo_user: a valid system username
:param str pinput: data to send to process's stdin
:param bool capture_output: capture process output or not
:rtype: tuple
:return: return code, command output
"""
dpath = None
code, output = exec_cmd("which doveadm")
code, output = exec_cmd(("which", "doveadm"))
if not code:
dpath = force_text(output).strip()
else:
Expand All @@ -74,7 +74,7 @@ def doveadm_cmd(params, sudo_user=None, pinput=None,
dpath = fpath
break
if dpath:
return exec_cmd("{} {}".format(dpath, params),
return exec_cmd([dpath] + list(params),
sudo_user=sudo_user,
pinput=pinput,
capture_output=capture_output,
Expand Down
8 changes: 3 additions & 5 deletions tests.py
View file
Open in desktop
Expand Up @@ -24,11 +24,9 @@ def test_silent(self):
dburl = "default:%s://%s:%s@%s/%s" \
% (self.dbtype, self.dbuser, self.dbpassword,
self.dbhost, self.projname)
cmd = (
"modoboa-admin.py deploy --collectstatic "
"--dburl %s --domain %s --admin-username admin %s"
% (dburl, "localhost", self.projname)
)
cmd = ("modoboa-admin.py", "deploy", "--collectstatic",
"--dburl", dburl, "--domain", "localhost",
"--admin-username", "admin", self.projname)
code, output = exec_cmd(cmd, cwd=self.workdir)
self.assertEqual(code, 0)

Expand Down