fuxino/simple_backup
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Find rsync in PATH and pass the SSH password only to rsync

Browse Source 
A missing /usr/bin/rsync ended in a traceback, possibly after --remove-before-backup had removed old backups, and left SSHPASS in the environment.
This commit is contained in:
Fuxino
2026-10-21 22:36:03 +02:00
parent ac59969d82
commit 9e6aadf7d3
4 changed files with 50 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
man/simple_backup.1
+1 -1
View File
@@ -209,7 +209,7 @@ 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 value other than 0 or 24).
rsync error (rsync returned a value other than 0 or 24, or rsync is not installed).
.TP
.B 5
SSH connection failed.
simple_backup/simple_backup.py
+20 -11
View File
@@ -229,7 +229,8 @@ class Backup:
self._exclude_path = ''
self._remote = False
self._ssh: paramiko.client.SSHClient | None = None
self._password_auth = False
# Passed to sshpass through rsync's environment only
self._password: str | None = None
self._removed_count = 0
def check_params(self, homedir: str = '') -> int:
@@ -558,8 +559,7 @@ class Backup:
ssh.connect(**host, username=self.ssh_user, timeout=SSH_TIMEOUT, password=password)
self._password_auth = True
os.environ['SSHPASS'] = password
self._password = password
return ssh
except GetPassWarning as e:
@@ -690,6 +690,16 @@ class Backup:
_notify('Starting backup...')
# Check before anything is removed (--remove-before-backup)
rsync = which('rsync')
if rsync is None:
logger.critical('rsync not found. Install rsync to perform the backup')
_notify('Backup failed (check log for details)')
return 4
self.define_backup_dir()
fd, self._inputs_path = mkstemp(prefix='tmp_inputs', text=True)
@@ -729,7 +739,7 @@ class Backup:
logger.info('Copying files. This may take a long time...')
# Build the argument list directly, so paths with spaces or quotes are passed unchanged
args = ['/usr/bin/rsync', *self.options.split()]
args = [rsync, *self.options.split()]
if self._last_backup != '':
args.append(f'--link-dest={self._last_backup}')
@@ -737,11 +747,15 @@ class Backup:
args.extend([f'--exclude-from={self._exclude_path}', f'--files-from={self._inputs_path}',
'/', f'{self._server}{self._output_dir}'])
# None inherits our environment
env: dict[str, str] | None = None
# 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 self.ssh_user is not None and which('sshpass'):
elif self._password is not None and self.ssh_user is not None and which('sshpass'):
ssh = f'sshpass -e ssh -l {shlex.quote(self.ssh_user)} -o StrictHostKeyChecking=accept-new'
env = {**os.environ, 'SSHPASS': self._password}
else:
ssh = 'ssh -o StrictHostKeyChecking=accept-new'
@@ -756,7 +770,7 @@ class Backup:
args.append('--info=progress2')
bar = _ProgressBar()
with Popen(args, stdin=PIPE, stdout=PIPE, stderr=STDOUT, shell=False) as p:
with Popen(args, stdin=PIPE, stdout=PIPE, stderr=STDOUT, shell=False, env=env) as p:
assert p.stdin is not None and isinstance(p.stdout, io.BufferedIOBase)
p.stdin.close()
@@ -770,11 +784,6 @@ class Backup:
if bar is not None:
bar.clear()
try:
del os.environ['SSHPASS']
except KeyError:
pass
returncode = p.wait()
if returncode == 0:
tests/test_local.py
+13 -1
View File
@@ -6,6 +6,7 @@ import os
from collections.abc import Callable, Iterator
from datetime import datetime, timedelta
from pathlib import Path
from shutil import which
from typing import Any
import pytest
@@ -338,10 +339,21 @@ class TestReturnCodes:
assert run_backup(out, [str(src)]) == 0
(args,) = fake_popen.calls
assert args[0] == '/usr/bin/rsync'
assert args[0] == which('rsync')
assert f'--link-dest={out}/simple_backup/{old_backups[-1]}' in args
assert args[args.index('/') + 1] == f'{out}/simple_backup/2026-10-14_10-00-00'
def test_rsync_missing(self, src: Path, out: Path, tmp_path: Path, fake_popen: type[FakePopen],
old_backups: list[str], monkeypatch: pytest.MonkeyPatch,
caplog: pytest.LogCaptureFixture) -> None:
monkeypatch.setenv('PATH', str(tmp_path / 'empty'))
assert run_backup(out, [str(src)], keep=0, remove_before=True) == 4
assert fake_popen.calls == []
# Nothing was removed
assert backups(out) == old_backups
assert 'rsync not found' in caplog.text
@pytest.mark.parametrize('returncode, result, removed', [
(0, 0, True),
(24, 0, True),
tests/test_remote.py
+16 -6
View File
@@ -254,9 +254,11 @@ class RecordPopen:
returncode = 0
calls: list[list[str]] = []
envs: list[dict[str, str] | None] = []
def __init__(self, args: list[str], **kwargs: Any) -> None:
RecordPopen.calls.append(args)
RecordPopen.envs.append(kwargs.get('env'))
self.stdin = io.BytesIO()
self.stdout = io.BytesIO(b'sent 1 bytes received 2 bytes\ntotal size is 3\n')
@@ -276,6 +278,7 @@ class TestRsyncArgs:
def record_popen(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
RecordPopen.returncode = 0
RecordPopen.calls = []
RecordPopen.envs = []
monkeypatch.setattr(sb, 'Popen', RecordPopen)
# The input must exist locally
(tmp_path / 'input').mkdir()
@@ -322,19 +325,26 @@ class TestRsyncArgs:
def test_password_auth(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setattr(sb, 'which', lambda cmd: f'/usr/bin/{cmd}')
monkeypatch.setenv('SSHPASS', 'secret')
monkeypatch.delenv('SSHPASS', raising=False)
backup = connected_backup()
backup.inputs = [str(tmp_path / 'input')]
backup._password_auth = True
backup._password = 'secret'
assert backup.run() == 0
(args,) = RecordPopen.calls
assert shlex.split(self.ssh_command(args)) == ['sshpass', '-e', 'ssh', '-l', 'user', '-o',
'StrictHostKeyChecking=accept-new']
# The password must not stay in the environment
# Only rsync gets the password, never our own environment
(env,) = RecordPopen.envs
assert env is not None and env['SSHPASS'] == 'secret'
assert 'SSHPASS' not in os.environ
def test_no_password_env(self, tmp_path: Path) -> None:
self.run(tmp_path)
assert RecordPopen.envs == [None]
def test_failure(self, tmp_path: Path, remote_home: Path) -> None:
base = make_backups(remote_home, '2026-10-01_10-00-00')
RecordPopen.returncode = 23
@@ -471,13 +481,13 @@ class TestLogin:
def test_password(self, client: FakeClient, monkeypatch: pytest.MonkeyPatch) -> None:
client.outcomes = [ssh_error(), ssh_error(), None]
monkeypatch.setattr(sb, 'getpass', lambda prompt: 'secret')
monkeypatch.setenv('SSHPASS', '')
monkeypatch.delenv('SSHPASS', raising=False)
backup = remote_backup()
assert backup._ssh_connect() is client
assert client.connects[-1]['password'] == 'secret'
assert backup._password_auth
assert os.environ['SSHPASS'] == 'secret'
assert backup._password == 'secret'
assert 'SSHPASS' not in os.environ
def test_wrong_password(self, client: FakeClient, monkeypatch: pytest.MonkeyPatch) -> None:
client.outcomes = [ssh_error(), ssh_error(), ssh_error()]