Update PKGBUILD

paramiko's connect had no timeout, so a server that doesn't answer
made the program hang until the OS gave up (about 2 minutes). Give up
after 15 seconds and exit with code 5.

PKGBUILD

@@ -4,7 +4,7 @@
 
 pkgname=simple_backup
 pkgdesc='Simple backup script that uses rsync to copy files'
-pkgver=4.1.7
+pkgver=4.3.0
 pkgrel=1
 url="https://git.shouldnt.work/fuxino/${pkgname}"
 arch=('any')
@@ -21,7 +21,7 @@ optdepends=('python-systemd: use systemd log'
 conflicts=('simple_backup-git')
 source=(git+${url}?signed#tag=${pkgver})
 validpgpkeys=('7E12BC1FF3B6EDB2CD8053EB981A2B2A3BBF5514')
-sha256sums=('011cd5546745e413ac7603b64531def2688c2bf6e4ea19876a67547a1db208ce')
+sha256sums=('7d9ea50d5829ac3c681a979745f1ae7afd2453eb46ceeae4f441ef5ae4540bd2')
 
 build()
 {

man/simple_backup.1

@@ -1,4 +1,4 @@
-.TH SIMPLE_BACKUP 1 2026-10-08 SIMPLE_BACKUP 4.2.0
+.TH SIMPLE_BACKUP 1 2026-10-08 SIMPLE_BACKUP 4.3.0
 .SH NAME
 simple_backup \- Backup files and folders using rsync
 .SH SYNOPSIS
@@ -146,6 +146,14 @@ Copy it to the default location ($HOME/.config/simple_backup) and edit it as nee
 It is possible to choose a directory on a remote server as destination for the backup. The files
 are copied by rsync through SSH. Server hostname and username must be specified, either in the
 configuration file, or on the command line (\(aq\-\-ssh\-host\(aq and \(aq\-\-ssh\-user\(aq options).
+.P
+The output directory is a path on the server. A leading \(aq~\(aq and relative paths refer to the
+home directory of the user on the server. On the command line, quote \(aq~\(aq so that the local
+shell doesn\(aqt expand it, for example:
+.P
+.EX
+    simple_backup \-\-ssh\-host server \-\-ssh\-user user \-o \(aq~/backups\(aq
+.EE
 .SS AUTHENTICATION
 For authentication, it is possible to use SSH key or password.
 .P
@@ -180,7 +188,8 @@ for details. For this reason, use SSH key authentication if possible.
 .SH EXIT STATUS
 .TP
 .B 0
-The backup was completed without errors.
+The backup was completed without errors. This includes rsync return code 24 (some source files
+vanished during the transfer), which only logs a warning.
 .TP
 .B 1
 No valid inputs selected for backup.
@@ -192,13 +201,13 @@ Backup failed because output directory for storing the backup does not exist.
 Permission denied to access the output directory.
 .TP
 .B 4
-rsync error (rsync returned a non-zero value).
+rsync error (rsync returned a value other than 0 or 24).
 .TP
 .B 5
 SSH connection failed.
 .TP
 .B 6
-Bad configuration file.
+Bad configuration file or options (for example, only one of SSH host and SSH user specified).
 .SH SEE ALSO
 .BR rsync (1)
 .SH AUTHORS

pyproject.toml

@@ -33,7 +33,7 @@ classifiers = [
 [project.optional-dependencies]
 JOURNAL = ["systemd-python"]
 NOTIFICATIONS = ["dbus-python"]
-REMOTE = ["paramiko"]
+REMOTE = ["paramiko>=3.2"]
 
 [project.urls]
 Homepage = "https://git.shouldnt.work/fuxino/simple_backup"

simple_backup/__init__.py

@@ -1,3 +1,3 @@
 """Init."""
 
-__version__ = '4.2.0'
+__version__ = '4.3.0'

simple_backup/simple_backup.conf

@@ -4,7 +4,7 @@
 # Files and directories to backup. Multiple items can be separated using a comma (','). It is possible to use wildcards (i.e. '*' to match multiple characters and '~' for the user's home directory).
 inputs=/home/user
 
-# Output directory.
+# Output directory. For remote backups, this is a path on the server ('~' and relative paths refer to the home directory on the server).
 backup_dir=/media/Backup
 
 # Files, directories and patterns to exclude from the backup. Multiple items can be separated using a comma.
@@ -18,5 +18,5 @@ keep=-1
 # ssh_host=
 # ssh_user=
 # ssh_keyfile=
-# remote_sudo=
+# remote_sudo=false
-# numeric_ids=
+# numeric_ids=false

simple_backup/simple_backup.py

@@ -36,7 +36,6 @@ from glob import glob
 
 try:
     import paramiko
-    from paramiko import RSAKey, Ed25519Key, ECDSAKey, DSSKey
 except ImportError:
     pass
 
@@ -70,6 +69,9 @@ if journal:
 P = ParamSpec('P')
 R = TypeVar('R')
 
+# Seconds to wait for the TCP connection to the SSH server
+SSH_TIMEOUT = 15
+
 
 def timing(func: Callable[P, R]) -> Callable[P, R]:
     """Decorator to measure execution time of a function
@@ -156,7 +158,6 @@ class Backup:
         self._remote = False
         self._ssh = None
         self._password_auth = False
-        self._password = None
         self._removed_count = 0
 
     def check_params(self, homedir: str = '') -> int:
@@ -178,20 +179,26 @@ class Backup:
             self._remote = True
 
         if self._remote:
+            try:
                 self._ssh = self._ssh_connect(homedir)
+            except OSError as e:
+                # Unknown hostname, connection refused, unreachable host or timeout
+                logger.critical('Can\'t connect to the server %s', self.ssh_host)
+                logger.critical(e)
+
+                return 5
 
             if self._ssh is None:
                 return 5
 
-            _, stdout, _ = self._ssh.exec_command(
+            output = self._resolve_remote_path(self.output)
-                f'if [ -d "{self.output}" ]; then echo "ok"; fi')
 
-            output = stdout.read().decode('utf-8').strip()
+            if output is None:
-
-            if output != 'ok':
                 logger.critical('Output path for backup does not exist')
 
                 return 2
+
+            self.output = output
         else:
             if not os.path.isdir(self.output):
                 logger.critical('Output path for backup does not exist')
@@ -205,6 +212,27 @@ class Backup:
 
         return 0
 
+    def _resolve_remote_path(self, path: str) -> Optional[str]:
+        """Return the absolute path of a directory on the server, or None if it doesn't exist"""
+
+        assert self._ssh is not None
+
+        # Leave a leading '~' unquoted, so that the server's shell expands it to the remote home.
+        # Relative paths are resolved from the remote home, where the command starts
+        if path == '~' or path.startswith('~/'):
+            target = f'~/{shlex.quote(path[2:])}'
+        else:
+            target = shlex.quote(path)
+
+        _, stdout, _ = self._ssh.exec_command(f'cd -- {target} && pwd')
+
+        output = stdout.read().decode('utf-8').strip().split('\n')[-1]
+
+        if not output.startswith('/'):
+            return None
+
+        return output
+
     # Function to create the actual backup directory
     def define_backup_dir(self) -> None:
         """Define the actual backup dir"""
@@ -222,10 +250,23 @@ class Backup:
         if self._remote:
             assert self._ssh is not None
 
-            _, stdout, _ = self._ssh.exec_command(
+            backup_dir = shlex.quote(self.output + '/simple_backup/')
-                f'ls {self.output}/simple_backup')
 
-            dirs = stdout.read().decode('utf-8').strip().split('\n')
+            # List only directories, and nothing (without errors) if no backup was made yet
+            _, stdout, stderr = self._ssh.exec_command(
+                f'if [ -d {backup_dir} ]; then find {backup_dir} -mindepth 1 -maxdepth 1 -type d; fi')
+
+            output = stdout.read().decode('utf-8')
+            err = stderr.read().decode('utf-8').strip()
+
+            if err != '':
+                # Don't risk removing the wrong directories if the list may be incomplete
+                logger.error('Cannot list old backups on the server. Old backups will not be removed')
+                logger.error(err)
+
+                return
+
+            dirs = [os.path.basename(d) for d in output.split('\n') if d != '']
 
             n_backup = len(dirs)
 
@@ -241,24 +282,30 @@ class Backup:
                 for i in range(n_backup - self.keep):
                     if self.remote_sudo:
                         _, _, stderr = self._ssh.exec_command(
-                            f'sudo rm -r "{self.output}/simple_backup/{dirs[i]}"')
+                            f'sudo rm -r {shlex.quote(f"{self.output}/simple_backup/{dirs[i]}")}')
                     else:
                         _, _, stderr = self._ssh.exec_command(
-                            f'rm -r "{self.output}/simple_backup/{dirs[i]}"')
+                            f'rm -r {shlex.quote(f"{self.output}/simple_backup/{dirs[i]}")}')
 
                     err = stderr.read().decode('utf-8').strip().split('\n')[0]
 
                     if err != '':
                         logger.error(
-                            'Error while removing backup %s.', {dirs[i]})
+                            'Error while removing backup %s.', dirs[i])
                         logger.error(err)
                     else:
                         count += 1
         else:
             try:
-                dirs = os.listdir(f'{self.output}/simple_backup')
+                # Only count directories, so that stray files don't count as backups
+                dirs = [f.name for f in os.scandir(f'{self.output}/simple_backup')
+                        if f.is_dir(follow_symlinks=False)]
             except FileNotFoundError:
                 return
+            except PermissionError:
+                logger.error('Cannot list old backups. Permission denied. Old backups will not be removed')
+
+                return
 
             n_backup = len(dirs)
 
@@ -298,7 +345,7 @@ class Backup:
                 sys.exit(5)
 
             _, stdout, _ = self._ssh.exec_command(
-                f'find {self.output}/simple_backup/ -mindepth 1 -maxdepth 1 -type d | sort')
+                f'find {shlex.quote(self.output + "/simple_backup/")} -mindepth 1 -maxdepth 1 -type d | sort')
             output = stdout.read().decode('utf-8').strip().split('\n')
 
             if output[-1] != '':
@@ -334,6 +381,18 @@ class Backup:
 
             return None
 
+        connected = None
+
+        try:
+            connected = self._ssh_login(ssh, homedir)
+
+            return connected
+        finally:
+            # Don't leave a half-open connection (e.g. after failed authentication)
+            if connected is None:
+                ssh.close()
+
+    def _ssh_login(self, ssh: paramiko.client.SSHClient, homedir: str) -> Optional[paramiko.client.SSHClient]:
         try:
             ssh.load_host_keys(filename=f'{homedir}/.ssh/known_hosts')
         except FileNotFoundError:
@@ -345,16 +404,22 @@ class Backup:
             # WarningPolicy emits a UserWarning for unknown host keys
             with warnings.catch_warnings():
                 warnings.simplefilter('error', UserWarning)
-                ssh.connect(self.ssh_host, username=self.ssh_user)
+                ssh.connect(self.ssh_host, username=self.ssh_user, timeout=SSH_TIMEOUT)
 
             return ssh
         except UserWarning:
+            try:
                 k = input(
                     f'Unknown key for host {self.ssh_host}. Continue anyway? (Y/N) ')
+            except EOFError:
+                # No terminal available (e.g. cron or systemd)
+                k = ''
 
-            if k[0].upper() == 'Y':
+            if k.strip().upper().startswith('Y'):
                 ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
             else:
+                logger.critical('Unknown host key for %s. Connection aborted', self.ssh_host)
+
                 return None
         except paramiko.BadHostKeyException as e:
             logger.critical('Can\'t connect to the server.')
@@ -365,7 +430,7 @@ class Backup:
             pass
 
         try:
-            ssh.connect(self.ssh_host, username=self.ssh_user)
+            ssh.connect(self.ssh_host, username=self.ssh_user, timeout=SSH_TIMEOUT)
 
             return ssh
         except paramiko.SSHException:
@@ -379,7 +444,7 @@ class Backup:
                     password = getpass(
                         f'{self.ssh_user}@{self.ssh_host}\'s password: ')
 
-                ssh.connect(self.ssh_host, username=self.ssh_user,
+                ssh.connect(self.ssh_host, username=self.ssh_user, timeout=SSH_TIMEOUT,
                             password=password)
 
                 self._password_auth = True
@@ -397,51 +462,13 @@ class Backup:
 
                 return None
 
-        pkey = None
+        pkey = self._load_ssh_key()
-
-        try:
-            pkey = RSAKey.from_private_key_file(self.ssh_keyfile)
-        except paramiko.PasswordRequiredException:
-            password = getpass(
-                f'Enter passwphrase for key \'{self.ssh_keyfile}\': ')
-
-            try:
-                pkey = RSAKey.from_private_key_file(self.ssh_keyfile, password)
-            except paramiko.SSHException:
-                pass
 
         if pkey is None:
-            try:
+            return None
-                pkey = Ed25519Key.from_private_key_file(self.ssh_keyfile)
-            except paramiko.PasswordRequiredException:
-                try:
-                    pkey = Ed25519Key.from_private_key_file(
-                        self.ssh_keyfile, password)
-                except paramiko.SSHException:
-                    pass
-
-        if pkey is None:
-            try:
-                pkey = ECDSAKey.from_private_key_file(self.ssh_keyfile)
-            except paramiko.PasswordRequiredException:
-                try:
-                    pkey = ECDSAKey.from_private_key_file(
-                        self.ssh_keyfile, password)
-                except paramiko.SSHException:
-                    pass
-
-        if pkey is None:
-            try:
-                pkey = DSSKey.from_private_key_file(self.ssh_keyfile)
-            except paramiko.PasswordRequiredException:
-                try:
-                    pkey = DSSKey.from_private_key_file(
-                        self.ssh_keyfile, password)
-                except paramiko.SSHException:
-                    pass
 
         try:
-            ssh.connect(self.ssh_host, username=self.ssh_user, pkey=pkey)
+            ssh.connect(self.ssh_host, username=self.ssh_user, timeout=SSH_TIMEOUT, pkey=pkey)
         except paramiko.SSHException:
             logger.critical('SSH connection to server failed')
 
@@ -449,6 +476,37 @@ class Backup:
 
         return ssh
 
+    def _load_ssh_key(self) -> Optional[paramiko.PKey]:
+        try:
+            return paramiko.PKey.from_path(self.ssh_keyfile)
+        except TypeError:
+            # Raised when the key is encrypted and no passphrase was given
+            pass
+        except (OSError, ValueError, paramiko.UnknownKeyType) as e:
+            logger.critical('Unable to load SSH key %s', self.ssh_keyfile)
+            logger.critical(e)
+
+            return None
+
+        try:
+            # Fail instead of reading the passphrase with echo if no terminal is available
+            with warnings.catch_warnings():
+                warnings.simplefilter('error', GetPassWarning)
+                passphrase = getpass(
+                    f'Enter passphrase for key \'{self.ssh_keyfile}\': ')
+        except GetPassWarning as e:
+            logger.critical('Unable to get passphrase')
+            logger.critical(e)
+
+            return None
+
+        try:
+            return paramiko.PKey.from_path(self.ssh_keyfile, passphrase.encode())
+        except (ValueError, paramiko.SSHException):
+            logger.critical('Unable to load SSH key %s, wrong passphrase?', self.ssh_keyfile)
+
+            return None
+
     def _returncode_log(self, returncode: int) -> None:
         match returncode:
             case 2:
@@ -480,7 +538,7 @@ class Backup:
                     'Rsync error (return code 23) - Partial transfer due to error')
             case 24:
                 logger.warning(
-                    'Rsync error (return code 24) - Partial transfer due to vanished source files')
+                    'Rsync warning (return code 24) - Some source files vanished during the transfer')
             case 30:
                 logger.error(
                     'Rsync error (return code 30) - Timeout in data send/receive')
@@ -501,6 +559,13 @@ class Backup:
         finally:
             self._remove_temp_files()
 
+    def close(self) -> None:
+        """Close the SSH connection, if open"""
+
+        if self._ssh is not None:
+            self._ssh.close()
+            self._ssh = None
+
     def _remove_temp_files(self) -> None:
         for path in [self._inputs_path, self._exclude_path]:
             if path != '':
@@ -553,26 +618,27 @@ class Backup:
 
         logger.info('Copying files. This may take a long time...')
 
-        if self._last_backup == '':
+        # Build the argument list directly, so paths with spaces or quotes are passed unchanged
-            rsync = f'/usr/bin/rsync {self.options} --exclude-from={self._exclude_path} ' +\
+        args = ['/usr/bin/rsync', *self.options.split()]
-                f'--files-from={self._inputs_path} / "{self._server}{self._output_dir}"'
-        else:
-            rsync = f'/usr/bin/rsync {self.options} --link-dest="{self._last_backup}" --exclude-from=' +\
-                f'{self._exclude_path} --files-from={self._inputs_path} / "{self._server}{self._output_dir}"'
 
-        euid = os.geteuid()
+        if self._last_backup != '':
+            args.append(f'--link-dest={self._last_backup}')
 
-        if euid == 0 and self.ssh_keyfile is not None:
+        args.extend([f'--exclude-from={self._exclude_path}', f'--files-from={self._inputs_path}',
-            rsync = f'{rsync} -e \'ssh -i {self.ssh_keyfile} -o StrictHostKeyChecking=no\''
+                     '/', f'{self._server}{self._output_dir}'])
+
+        # rsync splits the -e command itself, so quote the parts that may contain spaces
+        if self.ssh_keyfile is not None:
+            ssh = f'ssh -i {shlex.quote(self.ssh_keyfile)} -o StrictHostKeyChecking=accept-new'
         elif self._password_auth and which('sshpass'):
-            rsync = f'{rsync} -e \'sshpass -e ssh -l {self.ssh_user} -o StrictHostKeyChecking=no\''
+            ssh = f'sshpass -e ssh -l {shlex.quote(self.ssh_user)} -o StrictHostKeyChecking=accept-new'
         else:
-            rsync = f'{rsync} -e \'ssh -o StrictHostKeyChecking=no\''
+            ssh = 'ssh -o StrictHostKeyChecking=accept-new'
+
+        args.extend(['-e', ssh])
 
         if self._remote and self.remote_sudo:
-            rsync = f'{rsync} --rsync-path="sudo rsync"'
+            args.append('--rsync-path=sudo rsync')
-
-        args = shlex.split(rsync)
 
         with Popen(args, stdin=PIPE, stdout=PIPE, stderr=STDOUT, shell=False) as p:
             output: Union[bytes, List[str]]
@@ -620,27 +686,8 @@ class Backup:
             else:
                 logger.warning('Backup not completed successfully. Old backups will not be removed')
 
-        if self._remote:
+        # Files vanishing during the transfer is normal for a live system (return code 24)
-            assert self._ssh is not None
+        if returncode not in [0, 24]:
-
-            _, stdout, _ = self._ssh.exec_command(
-                f'if [ -d "{self._output_dir}" ]; then echo "ok"; fi')
-
-            output = stdout.read().decode('utf-8').strip()
-
-            if output == 'ok':
-                logger.info('Backup completed')
-
-                _notify('Backup completed')
-            else:
-                logger.error('Backup failed')
-
-                _notify('Backup failed (check log for details)')
-
-            if self._ssh:
-                self._ssh.close()
-        else:
-            if returncode != 0:
             logger.error(
                 'Some errors occurred while performing the backup')
 
@@ -782,14 +829,8 @@ def _read_config(config_file, user: Optional[str] = None):
 
     config_args['inputs'] = inputs
 
-    output = config.get(section, 'backup_dir')
+    # '~' is expanded later, since for remote backups it refers to the home on the server
-
+    config_args['output'] = config.get(section, 'backup_dir')
-    if user is not None:
-        output = _expand_home(output, user)
-    elif user is None and output.startswith('~'):
-        logger.warning('Cannot expand \'~\', no user specified')
-
-    config_args['output'] = output
 
     try:
         exclude = config.get(section, 'exclude')
@@ -806,36 +847,19 @@ def _read_config(config_file, user: Optional[str] = None):
 
     config_args['keep'] = keep
 
-    try:
+    # Read host and user separately, so that a missing one is reported instead of silently
-        ssh_host = config.get('server', 'ssh_host')
+    # disabling remote backup. Empty values count as not set
-        ssh_user = config.get('server', 'ssh_user')
+    config_args['ssh_host'] = config.get('server', 'ssh_host', fallback='').strip() or None
-    except (configparser.NoSectionError, configparser.NoOptionError):
+    config_args['ssh_user'] = config.get('server', 'ssh_user', fallback='').strip() or None
-        ssh_host = None
-        ssh_user = None
 
-    config_args['ssh_host'] = ssh_host
+    config_args['ssh_keyfile'] = config.get('server', 'ssh_keyfile', fallback='').strip() or None
-    config_args['ssh_user'] = ssh_user
 
-    try:
+    # Empty values (as in the example config) count as false
-        ssh_keyfile = config.get('server', 'ssh_keyfile')
+    for key in ['remote_sudo', 'numeric_ids']:
-    except (configparser.NoSectionError, configparser.NoOptionError):
+        if config.get('server', key, fallback='').strip() == '':
-        ssh_keyfile = None
+            config_args[key] = False
-
+        else:
-    config_args['ssh_keyfile'] = ssh_keyfile
+            config_args[key] = config.getboolean('server', key)
-
-    try:
-        remote_sudo = config.getboolean('server', 'remote_sudo')
-    except (configparser.NoSectionError, configparser.NoOptionError):
-        remote_sudo = False
-
-    config_args['remote_sudo'] = remote_sudo
-
-    try:
-        numeric_ids = config.getboolean('server', 'numeric_ids')
-    except (configparser.NoSectionError, configparser.NoOptionError):
-        numeric_ids = False
-
-    config_args['numeric_ids'] = numeric_ids
 
     return config_args
 
@@ -942,11 +966,30 @@ def simple_backup() -> int:
         return 6
 
     inputs = args.inputs if args.inputs is not None else config_args['inputs']
-    output = args.output if args.output is not None else config_args['output']
     exclude = args.exclude if args.exclude is not None else config_args['exclude']
     keep = args.keep if args.keep is not None else config_args['keep']
     ssh_host = args.ssh_host if args.ssh_host is not None else config_args['ssh_host']
     ssh_user = args.ssh_user if args.ssh_user is not None else config_args['ssh_user']
+
+    if (ssh_host is None) != (ssh_user is None):
+        logger.critical('Both SSH host and SSH user are needed for remote backup')
+
+        return 6
+
+    if args.output is not None:
+        output = args.output
+    else:
+        output = config_args['output']
+
+        # For remote backups, '~' is expanded on the server
+        remote = ssh_host is not None and ssh_user is not None
+
+        if output is not None and not remote:
+            if user is not None:
+                output = _expand_home(output, user)
+            elif output.startswith('~'):
+                logger.warning('Cannot expand \'~\', no user specified')
+
     ssh_keyfile = args.keyfile if args.keyfile is not None else config_args['ssh_keyfile']
     remote_sudo = args.remote_sudo or config_args['remote_sudo']
 
@@ -973,12 +1016,15 @@ def simple_backup() -> int:
     backup = Backup(inputs, output, exclude, keep, rsync_options, ssh_host, ssh_user, ssh_keyfile,
                     remote_sudo, remove_before=args.remove_before_backup, verbose=args.verbose)
 
+    try:
         return_code = backup.check_params(homedir)
 
         if return_code == 0:
             return backup.run()
 
         return return_code
+    finally:
+        backup.close()
 
 
 if __name__ == '__main__':