fix: harden backup module config coercion (#226)

Co-authored-by: Dennis Braun <itsDNNS@users.noreply.github.com>

Author: itsDNNS

app/modules/backup/collector.py

@@ -13,8 +13,27 @@ class BackupCollector(Collector):
     name = "backup"
 
     def __init__(self, config_mgr, poll_interval=86400, **kwargs):
-        super().__init__(poll_interval)
         self._config_mgr = config_mgr
+        interval_hours = self._get_interval_hours(config_mgr, poll_interval // 3600)
+        super().__init__(interval_hours * 3600)
+
+    @staticmethod
+    def _get_interval_hours(config_mgr, default_hours=24):
+        """Return configured backup interval in hours as int."""
+        try:
+            value = int(config_mgr.get("backup_interval_hours", default_hours))
+        except (TypeError, ValueError):
+            return default_hours
+        return value if value > 0 else default_hours
+
+    @staticmethod
+    def _get_retention(config_mgr, default_keep=5):
+        """Return backup retention as int."""
+        try:
+            value = int(config_mgr.get("backup_retention", default_keep))
+        except (TypeError, ValueError):
+            return default_keep
+        return value if value > 0 else default_keep
 
     def is_enabled(self):
         return self._config_mgr.is_backup_configured()
@@ -24,7 +43,7 @@ def collect(self):
 
         data_dir = self._config_mgr.data_dir
         backup_path = self._config_mgr.get("backup_path", "/backup")
-        retention = self._config_mgr.get("backup_retention", 5)
+        retention = self._get_retention(self._config_mgr)
 
         try:
             filename = create_backup_to_file(data_dir, backup_path)

app/modules/backup/routes.py

@@ -20,6 +20,15 @@
 bp = Blueprint("backup_bp", __name__)
 
 
+def _int_config(config_mgr, key, default):
+    """Read an integer-like config value safely."""
+    try:
+        value = int(config_mgr.get(key, default))
+    except (TypeError, ValueError):
+        return default
+    return value if value > 0 else default
+
+
 @bp.route("/api/backup", methods=["POST"])
 @require_auth
 def api_backup_download():
@@ -47,7 +56,7 @@ def api_backup_scheduled():
     if not _config_manager:
         return jsonify({"error": "Not initialized"}), 500
     backup_path = _config_manager.get("backup_path", "/backup")
-    retention = _config_manager.get("backup_retention", 5)
+    retention = _int_config(_config_manager, "backup_retention", 5)
     try:
         from .backup import create_backup_to_file, cleanup_old_backups
         filename = create_backup_to_file(_config_manager.data_dir, backup_path)

tests/test_backup.py

@@ -4,6 +4,7 @@
 import os
 import sqlite3
 import tarfile
+from unittest.mock import MagicMock
 
 import pytest
 from io import BytesIO
@@ -20,6 +21,7 @@
     restore_backup,
     validate_backup,
 )
+from app.modules.backup.collector import BackupCollector
 
 
 # ── Fixtures ──
@@ -264,6 +266,46 @@ def test_cleanup_noop_when_few(self, backup_dir):
         assert deleted == 0
 
 
+class TestBackupCollectorConfig:
+    def test_uses_configured_interval_hours_from_string(self):
+        mgr = MagicMock()
+        mgr.get.side_effect = lambda key, default=None: {
+            "backup_interval_hours": "168",
+        }.get(key, default)
+
+        collector = BackupCollector(mgr)
+
+        assert collector.poll_interval_seconds == 168 * 3600
+
+    def test_collect_casts_retention_from_string(self, monkeypatch):
+        mgr = MagicMock()
+        mgr.data_dir = "/data"
+        mgr.get.side_effect = lambda key, default=None: {
+            "backup_path": "/backup",
+            "backup_retention": "5",
+        }.get(key, default)
+
+        calls = {}
+
+        def fake_create_backup_to_file(data_dir, backup_path):
+            calls["create"] = (data_dir, backup_path)
+            return "docsight_backup_test.tar.gz"
+
+        def fake_cleanup_old_backups(backup_path, keep=5):
+            calls["cleanup"] = (backup_path, keep)
+            return 0
+
+        monkeypatch.setattr("app.modules.backup.backup.create_backup_to_file", fake_create_backup_to_file)
+        monkeypatch.setattr("app.modules.backup.backup.cleanup_old_backups", fake_cleanup_old_backups)
+
+        collector = BackupCollector(mgr)
+        result = collector.collect()
+
+        assert result.success is True
+        assert calls["create"] == ("/data", "/backup")
+        assert calls["cleanup"] == ("/backup", 5)
+
+
 # ── TestBrowseDirectory ──