From 25d2a5b1a7d782c0745a90b155c11f857c54adbc Mon Sep 17 00:00:00 2001 From: Peter van Arkel Date: Tue, 19 May 2026 19:49:33 +0200 Subject: [PATCH] (bugfix) Surface rsync SSH failure details in run results Include the selected SSH credential metadata and rsync log tail in dry-run and failed backup results so Django shows the actual SSH or rsync failure instead of only the exit code. Warn in host checks when a host still uses database-stored private key material, making it easier to spot old credentials after switching to generated filesystem keys. --- src/pobsync/commands/run_scheduled.py | 19 ++++++++++++++++++- src/pobsync_backend/config_source.py | 7 +++++++ src/pobsync_backend/host_ops.py | 9 +++++++++ .../tests/test_django_config_source.py | 2 ++ .../tests/test_run_scheduled_config_source.py | 18 ++++++++++++++++++ 5 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/pobsync/commands/run_scheduled.py b/src/pobsync/commands/run_scheduled.py index 014552e..ef68c3f 100644 --- a/src/pobsync/commands/run_scheduled.py +++ b/src/pobsync/commands/run_scheduled.py @@ -21,6 +21,14 @@ from ..snapshot_meta import read_snapshot_meta from ..util import ensure_dir, realpath_startswith, sanitize_host, write_yaml_atomic +def _read_log_tail(log_path: Path, *, max_lines: int = 40) -> list[str]: + try: + lines = log_path.read_text(encoding="utf-8", errors="replace").splitlines() + except OSError: + return [] + return lines[-max_lines:] + + def _host_backup_dirs(backup_root: str, host: str) -> HostBackupDirs: return HostBackupDirs(root=Path(backup_root) / host) @@ -159,6 +167,7 @@ def run_scheduled( ) result = run_rsync(cmd, log_path=dryrun_log, timeout_seconds=timeout_seconds) + log_tail = _read_log_tail(dryrun_log) return { "ok": result.exit_code == 0, @@ -166,9 +175,11 @@ def run_scheduled( "host": host, "base": str(base_dir) if base_dir else None, "log": str(dryrun_log), + "ssh_credential": cfg.get("ssh_credential"), "rsync": { "exit_code": result.exit_code, "command": result.command, + "log_tail": log_tail, }, } @@ -258,7 +269,13 @@ def run_scheduled( "host": host, "snapshot": str(incomplete_dir), "status": meta["status"], - "rsync": {"exit_code": result.exit_code}, + "log": str(log_path), + "ssh_credential": cfg.get("ssh_credential"), + "rsync": { + "exit_code": result.exit_code, + "command": result.command, + "log_tail": _read_log_tail(log_path), + }, } final_dir = dirs.scheduled / snap_name diff --git a/src/pobsync_backend/config_source.py b/src/pobsync_backend/config_source.py index d1bbd68..ff4003e 100644 --- a/src/pobsync_backend/config_source.py +++ b/src/pobsync_backend/config_source.py @@ -41,6 +41,13 @@ def _attach_credential_options(config: dict[str, Any], credential: SshCredential if paths.get("known_hosts") and not _has_ssh_option(options, "UserKnownHostsFile"): options.append(f"-oUserKnownHostsFile={paths['known_hosts']}") ssh["options"] = options + config["ssh_credential"] = { + "id": credential.pk, + "name": credential.name, + "identity_file": paths["identity_file"], + "generated": credential.generated, + "storage": "filesystem" if credential.key_path else "database", + } def _materialize_credential(credential: SshCredential) -> dict[str, str]: diff --git a/src/pobsync_backend/host_ops.py b/src/pobsync_backend/host_ops.py index e93e5f4..557dd2c 100644 --- a/src/pobsync_backend/host_ops.py +++ b/src/pobsync_backend/host_ops.py @@ -53,6 +53,15 @@ def collect_host_checks(host: HostConfig, global_config: GlobalConfig | None = N checks.append( _host_path_check("Host SSH key file", key_path, must_exist=True, must_be_writable=False, must_be_readable=True) ) + elif credential.private_key: + checks.append( + SelfCheck( + "Host SSH key storage", + "warning", + "Selected credential stores private key material in the database.", + "Generated filesystem keys are recommended for native systemd installs.", + ) + ) host_root = resolve_host_root(global_config.backup_root, host.host) checks.append(_host_path_check("Host backup root", host_root, must_exist=True, must_be_writable=True)) diff --git a/src/pobsync_backend/tests/test_django_config_source.py b/src/pobsync_backend/tests/test_django_config_source.py index 1fe0887..a65a431 100644 --- a/src/pobsync_backend/tests/test_django_config_source.py +++ b/src/pobsync_backend/tests/test_django_config_source.py @@ -90,6 +90,7 @@ class DjangoConfigSourceTests(TestCase): self.assertIn("-oBatchMode=yes", cfg["ssh"]["options"]) self.assertIn(f"-oIdentityFile={identity_file}", cfg["ssh"]["options"]) self.assertIn(f"-oUserKnownHostsFile={known_hosts}", cfg["ssh"]["options"]) + self.assertEqual(cfg["ssh_credential"]["storage"], "database") def test_host_ssh_credential_overrides_global_credential(self) -> None: global_credential = SshCredential.objects.create(name="global-key", private_key="GLOBAL") @@ -133,3 +134,4 @@ class DjangoConfigSourceTests(TestCase): cfg = DjangoConfigSource().effective_config_for_host("web-01") self.assertIn(f"-oIdentityFile={identity_file}", cfg["ssh"]["options"]) + self.assertEqual(cfg["ssh_credential"]["storage"], "filesystem") diff --git a/src/pobsync_backend/tests/test_run_scheduled_config_source.py b/src/pobsync_backend/tests/test_run_scheduled_config_source.py index 790bf5a..29c22ae 100644 --- a/src/pobsync_backend/tests/test_run_scheduled_config_source.py +++ b/src/pobsync_backend/tests/test_run_scheduled_config_source.py @@ -49,6 +49,24 @@ class RunScheduledConfigSourceTests(SimpleTestCase): self.assertEqual(result["host"], "web-01") run_rsync.assert_called_once() + def test_failed_dry_run_includes_log_tail(self) -> None: + def fake_run_rsync(command, log_path, timeout_seconds): + log_path.parent.mkdir(parents=True, exist_ok=True) + log_path.write_text("Permission denied (publickey).\nrsync error\n", encoding="utf-8") + return RsyncResult(exit_code=12, command=command) + + with patch("pobsync.commands.run_scheduled.run_rsync", side_effect=fake_run_rsync): + result = run_scheduled( + prefix=Path("/missing-prefix"), + host="web-01", + dry_run=True, + config_source=FakeConfigSource(), + ) + + self.assertFalse(result["ok"]) + self.assertEqual(result["rsync"]["exit_code"], 12) + self.assertEqual(result["rsync"]["log_tail"], ["Permission denied (publickey).", "rsync error"]) + def test_successful_real_run_applies_prune_when_requested(self) -> None: with TemporaryDirectory() as tmp: prefix = Path(tmp) / "home"