From 159e2bb734d951861c6048935c3a6e8290b92eb2 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Sun, 10 May 2020 05:46:10 -0700 Subject: [PATCH] flatpak: Build commands as lists instead of strings (#269) Using a list ensures that all subprocess arguments are correctly escaped. By building strings and then calling .split(), potential arguments with a space will be incorrectly split over two arguments. When a string is needed for presentation, join to the list to build the string. --- .../fragments/269-flatpak-command-list.yaml | 2 ++ plugins/modules/packaging/os/flatpak.py | 19 +++++++++---------- .../modules/packaging/os/flatpak_remote.py | 13 +++++-------- 3 files changed, 16 insertions(+), 18 deletions(-) create mode 100644 changelogs/fragments/269-flatpak-command-list.yaml diff --git a/changelogs/fragments/269-flatpak-command-list.yaml b/changelogs/fragments/269-flatpak-command-list.yaml new file mode 100644 index 0000000000..5ff5ae3567 --- /dev/null +++ b/changelogs/fragments/269-flatpak-command-list.yaml @@ -0,0 +1,2 @@ +bugfixes: +- flatpak and flatpak_remote - fix command line construction to build commands as lists instead of strings. diff --git a/plugins/modules/packaging/os/flatpak.py b/plugins/modules/packaging/os/flatpak.py index ce9e6fc66f..de088596b2 100644 --- a/plugins/modules/packaging/os/flatpak.py +++ b/plugins/modules/packaging/os/flatpak.py @@ -159,9 +159,9 @@ def install_flat(module, binary, remote, name, method): """Add a new flatpak.""" global result if name.startswith('http://') or name.startswith('https://'): - command = "{0} install --{1} -y {2}".format(binary, method, name) + command = [binary, "install", "--{0}".format(method), "-y", name] else: - command = "{0} install --{1} -y {2} {3}".format(binary, method, remote, name) + command = [binary, "install", "--{0}".format(method), "-y", remote, name] _flatpak_command(module, module.check_mode, command) result['changed'] = True @@ -170,14 +170,14 @@ def uninstall_flat(module, binary, name, method): """Remove an existing flatpak.""" global result installed_flat_name = _match_installed_flat_name(module, binary, name, method) - command = "{0} uninstall -y --{1} {2}".format(binary, method, installed_flat_name) + command = [binary, "uninstall", "--{0}".format(method), "-y", name] _flatpak_command(module, module.check_mode, command) result['changed'] = True def flatpak_exists(module, binary, name, method): """Check if the flatpak is installed.""" - command = "{0} list --{1} --app".format(binary, method) + command = [binary, "list", "--{0}".format(method), "--app"] output = _flatpak_command(module, False, command) name = _parse_flatpak_name(name).lower() if name in output.lower(): @@ -192,7 +192,7 @@ def _match_installed_flat_name(module, binary, name, method): global result parsed_name = _parse_flatpak_name(name) # Try running flatpak list with columns feature - command = "{0} list --{1} --app --columns=application".format(binary, method) + command = [binary, "list", "--{0}".format(method), "--app", "--columns=application"] _flatpak_command(module, False, command, ignore_failure=True) if result['rc'] != 0 and OUTDATED_FLATPAK_VERSION_ERROR_MESSAGE in result['stderr']: # Probably flatpak before 1.2 @@ -214,7 +214,7 @@ def _match_installed_flat_name(module, binary, name, method): def _match_flat_using_outdated_flatpak_format(module, binary, parsed_name, method): global result - command = "{0} list --{1} --app --columns=application".format(binary, method) + command = [binary, "list", "--{0}".format(method), "--app", "--columns=application"] output = _flatpak_command(module, False, command) for row in output.split('\n'): if parsed_name.lower() == row.lower(): @@ -223,7 +223,7 @@ def _match_flat_using_outdated_flatpak_format(module, binary, parsed_name, metho def _match_flat_using_flatpak_column_feature(module, binary, parsed_name, method): global result - command = "{0} list --{1} --app".format(binary, method) + command = [binary, "list", "--{0}".format(method), "--app"] output = _flatpak_command(module, False, command) for row in output.split('\n'): if parsed_name.lower() in row.lower(): @@ -242,15 +242,14 @@ def _parse_flatpak_name(name): def _flatpak_command(module, noop, command, ignore_failure=False): global result + result['command'] = ' '.join(command) if noop: result['rc'] = 0 - result['command'] = command return "" result['rc'], result['stdout'], result['stderr'] = module.run_command( - command.split(), check_rc=not ignore_failure + command, check_rc=not ignore_failure ) - result['command'] = command return result['stdout'] diff --git a/plugins/modules/packaging/os/flatpak_remote.py b/plugins/modules/packaging/os/flatpak_remote.py index edd9da9af6..e9006dfb9a 100644 --- a/plugins/modules/packaging/os/flatpak_remote.py +++ b/plugins/modules/packaging/os/flatpak_remote.py @@ -150,8 +150,7 @@ from ansible.module_utils._text import to_bytes, to_native def add_remote(module, binary, name, flatpakrepo_url, method): """Add a new remote.""" global result - command = "{0} remote-add --{1} {2} {3}".format( - binary, method, name, flatpakrepo_url) + command = [binary, "remote-add", "--{0}".format(method), name, flatpakrepo_url] _flatpak_command(module, module.check_mode, command) result['changed'] = True @@ -159,15 +158,14 @@ def add_remote(module, binary, name, flatpakrepo_url, method): def remove_remote(module, binary, name, method): """Remove an existing remote.""" global result - command = "{0} remote-delete --{1} --force {2} ".format( - binary, method, name) + command = [binary, "remote-delete", "--{0}".format(method), "--force", name] _flatpak_command(module, module.check_mode, command) result['changed'] = True def remote_exists(module, binary, name, method): """Check if the remote exists.""" - command = "{0} remote-list -d --{1}".format(binary, method) + command = [binary, "remote-list", "-d", "--{0}".format(method)] # The query operation for the remote needs to be run even in check mode output = _flatpak_command(module, False, command) for line in output.splitlines(): @@ -181,15 +179,14 @@ def remote_exists(module, binary, name, method): def _flatpak_command(module, noop, command): global result + result['command'] = ' '.join(command) if noop: result['rc'] = 0 - result['command'] = command return "" result['rc'], result['stdout'], result['stderr'] = module.run_command( - command.split(), check_rc=True + command, check_rc=True ) - result['command'] = command return result['stdout']