From b8fd4cf375add2031460c2f9d1fc91de62890e38 Mon Sep 17 00:00:00 2001
From: Anton Khirnov <anton@khirnov.net>
Date: Mon, 16 Oct 2023 17:17:03 +0200
Subject: targets: shell-escape commands executed over SSH

---
 lbup/targets.py | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/lbup/targets.py b/lbup/targets.py
index 3473ed1..4127b47 100644
--- a/lbup/targets.py
+++ b/lbup/targets.py
@@ -5,6 +5,7 @@ import errno
 import logging
 import re
 import secrets
+import shlex
 import socket
 import subprocess
 
@@ -172,6 +173,7 @@ class TargetSSH(Target):
         return "%s{SSH:%s}" % (super().__str__(), str(self._remote))
 
     def _paramiko_exec_cmd(self, client, cmd, decode = True):
+        cmd = shlex.join(map(str, cmd))
         self._logger.debug('Client %s: executing command: %s' % (client, cmd))
 
         res = client.exec_command(cmd)
@@ -197,7 +199,7 @@ class TargetSSH(Target):
         return out
 
     def _resolve_remote_path(self, ssh, path):
-        path = self._paramiko_exec_cmd(ssh, 'realpath -e %s' % path).splitlines()
+        path = self._paramiko_exec_cmd(ssh, ['realpath', '-e', path]).splitlines()
         if len(path) != 1:
             raise BackupException('Expected exactly one path from realpath', path)
         return AbsPath(path[0])
@@ -242,8 +244,8 @@ class TargetSSHLVM(TargetSSH):
         major = devnum >> 8
         minor = devnum & 255
         res = self._paramiko_exec_cmd(ssh,
-            'lvs --select "kernel_major={major}&&kernel_minor={minor}" '
-            '--noheadings -o lv_full_name'.format(major = major, minor = minor))
+            ['lvs', '--select', 'kernel_major=%d&&kernel_minor=%d' % (major, minor),
+             '--noheadings', '-o', 'lv_full_name'])
 
         lv_name = res.strip()
         # valid LV paths are volname/lvname, each non-empty alphanumeric+_
@@ -268,14 +270,14 @@ class TargetSSHLVM(TargetSSH):
         snapshot_name     = 'a' + secrets.token_urlsafe()
         snapshot_fullname = '%s/%s' % (vg_name, snapshot_name)
         self._paramiko_exec_cmd(ssh,
-            'lvcreate --permission r --snapshot -L {size} -n {name} {origin}'
-            .format(size = self._snapshot_size, name = snapshot_name,
-                    origin = lv_fullname))
+            ['lvcreate', '--permission', 'r', '--snapshot', '-L', self._snapshot_size,
+             '-n', snapshot_name, lv_fullname])
 
         try:
             # get the path to the snapshot device node
             res = self._paramiko_exec_cmd(ssh,
-                'lvs --select "lv_full_name=%s" --noheadings -o lv_path' % snapshot_fullname)
+                    ['lvs', '--select', 'lv_full_name=%s' % snapshot_fullname,
+                     '--noheadings', '-o', 'lv_path'])
             lv_path = res.strip()
             if not lv_path.startswith('/'):
                 raise BackupException('Got invalid snapshot LV path', lv_path)
@@ -284,7 +286,7 @@ class TargetSSHLVM(TargetSSH):
 
             yield lv_path
         finally:
-            self._paramiko_exec_cmd(ssh, 'lvremove -f %s' % snapshot_fullname)
+            self._paramiko_exec_cmd(ssh, ['lvremove', '-f', snapshot_fullname])
             self._logger.debug('Removed snapshot %s', snapshot_fullname)
 
     @contextlib.contextmanager
@@ -295,13 +297,11 @@ class TargetSSHLVM(TargetSSH):
         then unmounts and destroys it at exit.
         """
         with self._snapshot_lv(ssh, devnum) as lv_path:
-            self._paramiko_exec_cmd(ssh,
-                'mount -t{fstype} -oro {lv_path} {mount_path}'.format(
-                    fstype = fstype, lv_path = lv_path, mount_path = mount_path))
+            self._paramiko_exec_cmd(ssh, ['mount', '-t', fstype, '-oro', lv_path, mount_path])
             try:
                 yield None
             finally:
-                self._paramiko_exec_cmd(ssh, 'umount %s' % mount_path)
+                self._paramiko_exec_cmd(ssh, ['umount', mount_path])
 
 
     def save(self, dry_run = False):
@@ -322,11 +322,12 @@ class TargetSSHLVM(TargetSSH):
             # same for each backup
             # we use BUP_DIR/lbup_mount
             snapshot_root = bupdir + 'lbup_mount'
-            self._paramiko_exec_cmd(conn_tgt, 'mkdir -p -m 700 ' + str(snapshot_root))
+            self._paramiko_exec_cmd(conn_tgt,
+                                    ['mkdir', '-p', '-m', '700', str(snapshot_root)])
 
             # read and parse the mountinfo table
             mntinfo  = MountInfo(
-                self._paramiko_exec_cmd(conn_root, 'cat /proc/1/mountinfo', decode = False))
+                self._paramiko_exec_cmd(conn_root, ['cat', '/proc/1/mountinfo'], decode = False))
 
             mounts = _mounts_for_dirs(mntinfo, dirs)
 
-- 
cgit v1.2.3