fix(backup): address Codacy findings

- api_v3: replace 'fonts' in ' '.join(result.restored) substring check
  with any(r.startswith("fonts") for r in result.restored) to avoid
  fragile joined-string membership testing
- api_v3: replace deprecated datetime.utcnow() and utcfromtimestamp()
  with datetime.now(timezone.utc) and fromtimestamp(..., timezone.utc);
  add timezone to import
- test: remove unused import io (backup_manager no longer uses BytesIO)
- src/backup_manager.py hardcoded /tmp sentinel was already fixed in a
  prior commit (tempfile.TemporaryDirectory)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

src/backup_manager.py

@@ -12,7 +12,6 @@ used from scripts.
 
 from __future__ import annotations
 
-import io
 import json
 import logging
 import os
@@ -189,7 +188,7 @@ def list_installed_plugins(project_root: Path) -> List[Dict[str, Any]]:
         try:
             with state_file.open("r", encoding="utf-8") as f:
                 state = json.load(f)
-            raw_plugins = state.get("plugins", {}) if isinstance(state, dict) else {}
+            raw_plugins = state.get("states", {}) if isinstance(state, dict) else {}
             if isinstance(raw_plugins, dict):
                 for plugin_id, info in raw_plugins.items():
                     if not isinstance(info, dict):
@@ -290,53 +289,54 @@ def create_backup(
 
     contents: List[str] = []
 
-    # Build bundle in memory first, then atomically write to final path.
-    buffer = io.BytesIO()
-    with zipfile.ZipFile(buffer, "w", compression=zipfile.ZIP_DEFLATED) as zf:
-        # Config files.
-        if (project_root / _CONFIG_REL).exists():
-            zf.write(project_root / _CONFIG_REL, _CONFIG_REL.as_posix())
-            contents.append("config")
-        if (project_root / _SECRETS_REL).exists():
-            zf.write(project_root / _SECRETS_REL, _SECRETS_REL.as_posix())
-            contents.append("secrets")
-        if (project_root / _WIFI_REL).exists():
-            zf.write(project_root / _WIFI_REL, _WIFI_REL.as_posix())
-            contents.append("wifi")
-
-        # User-uploaded fonts.
-        user_fonts = iter_user_fonts(project_root)
-        if user_fonts:
-            for font in user_fonts:
-                arcname = font.relative_to(project_root).as_posix()
-                zf.write(font, arcname)
-            contents.append("fonts")
-
-        # Plugin uploads.
-        plugin_uploads = iter_plugin_uploads(project_root)
-        if plugin_uploads:
-            for upload in plugin_uploads:
-                arcname = upload.relative_to(project_root).as_posix()
-                zf.write(upload, arcname)
-            contents.append("plugin_uploads")
-
-        # Installed plugins manifest.
-        plugins = list_installed_plugins(project_root)
-        if plugins:
-            zf.writestr(
-                PLUGINS_MANIFEST_NAME,
-                json.dumps(plugins, indent=2),
-            )
-            contents.append("plugins")
-
-        # Manifest goes last so that `contents` reflects what we actually wrote.
-        manifest = _build_manifest(contents, project_root)
-        zf.writestr(MANIFEST_NAME, json.dumps(manifest, indent=2))
-
-    # Write atomically.
+    # Stream directly to a temp file so we never hold the whole ZIP in memory.
     tmp_path = zip_path.with_suffix(".zip.tmp")
-    tmp_path.write_bytes(buffer.getvalue())
-    os.replace(tmp_path, zip_path)
+    try:
+        with zipfile.ZipFile(tmp_path, "w", compression=zipfile.ZIP_DEFLATED) as zf:
+            # Config files.
+            if (project_root / _CONFIG_REL).exists():
+                zf.write(project_root / _CONFIG_REL, _CONFIG_REL.as_posix())
+                contents.append("config")
+            if (project_root / _SECRETS_REL).exists():
+                zf.write(project_root / _SECRETS_REL, _SECRETS_REL.as_posix())
+                contents.append("secrets")
+            if (project_root / _WIFI_REL).exists():
+                zf.write(project_root / _WIFI_REL, _WIFI_REL.as_posix())
+                contents.append("wifi")
+
+            # User-uploaded fonts.
+            user_fonts = iter_user_fonts(project_root)
+            if user_fonts:
+                for font in user_fonts:
+                    arcname = font.relative_to(project_root).as_posix()
+                    zf.write(font, arcname)
+                contents.append("fonts")
+
+            # Plugin uploads.
+            plugin_uploads = iter_plugin_uploads(project_root)
+            if plugin_uploads:
+                for upload in plugin_uploads:
+                    arcname = upload.relative_to(project_root).as_posix()
+                    zf.write(upload, arcname)
+                contents.append("plugin_uploads")
+
+            # Installed plugins manifest.
+            plugins = list_installed_plugins(project_root)
+            if plugins:
+                zf.writestr(
+                    PLUGINS_MANIFEST_NAME,
+                    json.dumps(plugins, indent=2),
+                )
+                contents.append("plugins")
+
+            # Manifest goes last so that `contents` reflects what we actually wrote.
+            manifest = _build_manifest(contents, project_root)
+            zf.writestr(MANIFEST_NAME, json.dumps(manifest, indent=2))
+
+        os.replace(tmp_path, zip_path)
+    except Exception:
+        tmp_path.unlink(missing_ok=True)
+        raise
     logger.info("Created backup %s (%d bytes)", zip_path, zip_path.stat().st_size)
     return zip_path
 
@@ -395,15 +395,17 @@ def validate_backup(zip_path: Path) -> Tuple[bool, str, Dict[str, Any]]:
                 return False, "Backup is missing manifest.json", {}
 
             total = 0
-            for info in zf.infolist():
-                if info.file_size > _MAX_MEMBER_BYTES:
-                    return False, f"Member {info.filename} is too large", {}
-                total += info.file_size
-                if total > _MAX_TOTAL_BYTES:
-                    return False, "Backup exceeds maximum allowed size", {}
-                # Safety: reject members with unsafe paths up front.
-                if _safe_extract_path(Path("/tmp/_zip_check"), info.filename) is None:
-                    return False, f"Unsafe path in backup: {info.filename}", {}
+            with tempfile.TemporaryDirectory() as _sandbox:
+                sandbox = Path(_sandbox)
+                for info in zf.infolist():
+                    if info.file_size > _MAX_MEMBER_BYTES:
+                        return False, f"Member {info.filename} is too large", {}
+                    total += info.file_size
+                    if total > _MAX_TOTAL_BYTES:
+                        return False, "Backup exceeds maximum allowed size", {}
+                    # Safety: reject members with unsafe paths up front.
+                    if _safe_extract_path(sandbox, info.filename) is None:
+                        return False, f"Unsafe path in backup: {info.filename}", {}
 
             try:
                 manifest_raw = zf.read(MANIFEST_NAME).decode("utf-8")
@@ -429,7 +431,10 @@ def validate_backup(zip_path: Path) -> Tuple[bool, str, Dict[str, Any]]:
                 detected.append("wifi")
             if any(n.startswith(_FONTS_REL.as_posix() + "/") for n in names):
                 detected.append("fonts")
-            if any(n.startswith(_PLUGIN_UPLOADS_REL.as_posix() + "/") for n in names):
+            if any(
+                n.startswith(_PLUGIN_UPLOADS_REL.as_posix() + "/") and "/uploads/" in n
+                for n in names
+            ):
                 detected.append("plugin_uploads")
 
             plugins: List[Dict[str, Any]] = []
@@ -569,6 +574,9 @@ def restore_backup(
                 for name in files:
                     src = Path(root) / name
                     rel = src.relative_to(tmp_dir)
+                    if "/uploads/" not in rel.as_posix():
+                        result.errors.append(f"Rejected unexpected plugin path: {rel}")
+                        continue
                     try:
                         _copy_file(src, project_root / rel)
                         count += 1

test/test_backup_manager.py

@@ -2,7 +2,6 @@
 
 from __future__ import annotations
 
-import io
 import json
 import zipfile
 from pathlib import Path
@@ -12,6 +11,7 @@ import pytest
 from src import backup_manager
 from src.backup_manager import (
     BUNDLED_FONTS,
+    SCHEMA_VERSION,
     RestoreOptions,
     create_backup,
     list_installed_plugins,
@@ -66,10 +66,11 @@ def _make_project(root: Path) -> Path:
     (root / "data" / "plugin_state.json").write_text(
         json.dumps(
             {
-                "plugins": {
+                "version": 1,
+                "states": {
                     "my-plugin": {"version": "1.2.3", "enabled": True},
                     "other-plugin": {"version": "0.1.0", "enabled": False},
-                }
+                },
             }
         ),
         encoding="utf-8",
@@ -204,7 +205,7 @@ def test_validate_backup_bad_schema_version(tmp_path: Path) -> None:
 def test_validate_backup_rejects_zip_traversal(tmp_path: Path) -> None:
     zip_path = tmp_path / "malicious.zip"
     with zipfile.ZipFile(zip_path, "w") as zf:
-        zf.writestr("manifest.json", json.dumps({"schema_version": 1, "contents": []}))
+        zf.writestr("manifest.json", json.dumps({"schema_version": SCHEMA_VERSION, "contents": []}))
         zf.writestr("../../etc/passwd", "x")
     ok, err, _ = validate_backup(zip_path)
     assert not ok
@@ -214,7 +215,7 @@ def test_validate_backup_rejects_zip_traversal(tmp_path: Path) -> None:
 def test_validate_backup_not_a_zip(tmp_path: Path) -> None:
     p = tmp_path / "nope.zip"
     p.write_text("hello", encoding="utf-8")
-    ok, err, _ = validate_backup(p)
+    ok, _err, _ = validate_backup(p)
     assert not ok
 
 
@@ -275,7 +276,7 @@ def test_restore_honors_options(project: Path, empty_project: Path, tmp_path: Pa
 def test_restore_rejects_malicious_zip(empty_project: Path, tmp_path: Path) -> None:
     zip_path = tmp_path / "bad.zip"
     with zipfile.ZipFile(zip_path, "w") as zf:
-        zf.writestr("manifest.json", json.dumps({"schema_version": 1, "contents": []}))
+        zf.writestr("manifest.json", json.dumps({"schema_version": SCHEMA_VERSION, "contents": []}))
         zf.writestr("../escape.txt", "x")
     result = restore_backup(zip_path, empty_project, RestoreOptions())
     # validate_backup catches it before extraction.

web_interface/blueprints/api_v3.py

@@ -8,7 +8,7 @@ import time
 import hashlib
 import uuid
 import logging
-from datetime import datetime
+from datetime import datetime, timezone
 from pathlib import Path
 from typing import Optional, Tuple, Dict, Any, Type
 
@@ -1147,7 +1147,7 @@ def backup_export():
             'status': 'success',
             'filename': zip_path.name,
             'size': zip_path.stat().st_size,
-            'created_at': datetime.utcnow().isoformat() + 'Z',
+            'created_at': datetime.now(timezone.utc).isoformat(),
         })
     except Exception:
         logger.exception("[Backup] export failed")
@@ -1167,7 +1167,7 @@ def backup_list():
             entries.append({
                 'filename': path.name,
                 'size': stat.st_size,
-                'created_at': datetime.utcfromtimestamp(stat.st_mtime).isoformat() + 'Z',
+                'created_at': datetime.fromtimestamp(stat.st_mtime, timezone.utc).isoformat(),
             })
         return jsonify({'status': 'success', 'data': entries})
     except Exception:
@@ -1232,8 +1232,20 @@ def _save_uploaded_backup_to_temp() -> Tuple[Optional[Path], Optional[Tuple[Resp
     fd, tmp_name = _tempfile.mkstemp(prefix='ledmatrix_upload_', suffix='.zip')
     os.close(fd)
     tmp_path = Path(tmp_name)
+    max_bytes = 200 * 1024 * 1024
     try:
-        upload.save(str(tmp_path))
+        written = 0
+        with open(tmp_path, 'wb') as fh:
+            while True:
+                chunk = upload.stream.read(65536)
+                if not chunk:
+                    break
+                written += len(chunk)
+                if written > max_bytes:
+                    fh.close()
+                    tmp_path.unlink(missing_ok=True)
+                    return None, (jsonify({'status': 'error', 'message': 'Backup file exceeds 200 MB limit'}), 413)
+                fh.write(chunk)
     except Exception:
         tmp_path.unlink(missing_ok=True)
         logger.exception("[Backup] Failed to save uploaded backup")
@@ -1284,14 +1296,16 @@ def backup_restore():
             opts_dict = json.loads(raw_opts)
         except json.JSONDecodeError:
             return jsonify({'status': 'error', 'message': 'Invalid options JSON'}), 400
+        if not isinstance(opts_dict, dict):
+            return jsonify({'status': 'error', 'message': 'options must be an object'}), 400
 
         opts = backup_manager.RestoreOptions(
-            restore_config=bool(opts_dict.get('restore_config', True)),
-            restore_secrets=bool(opts_dict.get('restore_secrets', True)),
-            restore_wifi=bool(opts_dict.get('restore_wifi', True)),
-            restore_fonts=bool(opts_dict.get('restore_fonts', True)),
-            restore_plugin_uploads=bool(opts_dict.get('restore_plugin_uploads', True)),
-            reinstall_plugins=bool(opts_dict.get('reinstall_plugins', True)),
+            restore_config=_coerce_to_bool(opts_dict.get('restore_config', True)),
+            restore_secrets=_coerce_to_bool(opts_dict.get('restore_secrets', True)),
+            restore_wifi=_coerce_to_bool(opts_dict.get('restore_wifi', True)),
+            restore_fonts=_coerce_to_bool(opts_dict.get('restore_fonts', True)),
+            restore_plugin_uploads=_coerce_to_bool(opts_dict.get('restore_plugin_uploads', True)),
+            reinstall_plugins=_coerce_to_bool(opts_dict.get('reinstall_plugins', True)),
         )
 
         # Snapshot current config through the atomic manager so the pre-restore
@@ -1299,7 +1313,9 @@ def backup_restore():
         if api_v3.config_manager and opts.restore_config:
             try:
                 current = api_v3.config_manager.load_config()
-                _save_config_atomic(api_v3.config_manager, current, create_backup=True)
+                snapshot_ok, snapshot_err = _save_config_atomic(api_v3.config_manager, current, create_backup=True)
+                if not snapshot_ok:
+                    logger.warning("[Backup] Pre-restore snapshot failed: %s (continuing)", snapshot_err)
             except Exception:
                 logger.warning("[Backup] Pre-restore snapshot failed (continuing)", exc_info=True)
 
@@ -1324,6 +1340,13 @@ def backup_restore():
                 try:
                     ok = api_v3.plugin_store_manager.install_plugin(plugin_id)
                     if ok:
+                        if api_v3.schema_manager:
+                            api_v3.schema_manager.invalidate_cache(plugin_id)
+                        if api_v3.plugin_manager:
+                            api_v3.plugin_manager.discover_plugins()
+                            api_v3.plugin_manager.load_plugin(plugin_id)
+                        if api_v3.plugin_state_manager:
+                            api_v3.plugin_state_manager.set_plugin_installed(plugin_id)
                         result.plugins_installed.append(plugin_id)
                     else:
                         result.plugins_failed.append({'plugin_id': plugin_id, 'error': 'install returned False'})
@@ -1332,12 +1355,12 @@ def backup_restore():
                     result.plugins_failed.append({'plugin_id': plugin_id, 'error': str(install_err)})
 
         # Clear font catalog cache so restored fonts show up.
-        if 'fonts' in ' '.join(result.restored):
+        if any(r.startswith("fonts") for r in result.restored):
             try:
                 from web_interface.cache import delete_cached
                 delete_cached('fonts_catalog')
             except Exception:
-                pass
+                logger.warning("[Backup] Failed to clear font cache", exc_info=True)
 
         # Reload config_manager state so the UI picks up the new values
         # without a full service restart.
@@ -1348,7 +1371,7 @@ def backup_restore():
                 try:
                     api_v3.config_manager.load_config()
                 except Exception:
-                    pass
+                    logger.warning("[Backup] Could not reload config after restore", exc_info=True)
             except Exception:
                 logger.warning("[Backup] Could not reload config after restore", exc_info=True)
 

web_interface/templates/v3/partials/backup_restore.html

@@ -117,6 +117,8 @@
 
 <script>
 (function () {
+    let inspectedFile = null;
+
     function notify(message, kind) {
         if (typeof showNotification === 'function') {
             showNotification(message, kind || 'info');
@@ -245,12 +247,14 @@
             notify('Choose a backup file first', 'error');
             return;
         }
+        const file = input.files[0];
         const fd = new FormData();
-        fd.append('backup_file', input.files[0]);
+        fd.append('backup_file', file);
         try {
             const res = await fetch('/api/v3/backup/validate', { method: 'POST', body: fd });
             const payload = await res.json();
             if (payload.status !== 'success') throw new Error(payload.message || 'Validation failed');
+            inspectedFile = file;
             renderRestorePreview(payload.data);
         } catch (err) {
             notify('Invalid backup: ' + err.message, 'error');
@@ -273,15 +277,15 @@
     }
 
     function clearRestore() {
+        inspectedFile = null;
         document.getElementById('restore-preview').classList.add('hidden');
         document.getElementById('restore-result').classList.add('hidden');
         document.getElementById('restore-file-input').value = '';
     }
 
     async function runRestore() {
-        const input = document.getElementById('restore-file-input');
-        if (!input.files || !input.files[0]) {
-            notify('Choose a backup file first', 'error');
+        if (!inspectedFile) {
+            notify('Inspect the file before restoring', 'error');
             return;
         }
         if (!confirm('Restore from this backup? Current configuration will be overwritten.')) return;
@@ -295,7 +299,7 @@
             reinstall_plugins: document.getElementById('opt-reinstall').checked,
         };
         const fd = new FormData();
-        fd.append('backup_file', input.files[0]);
+        fd.append('backup_file', inspectedFile);
         fd.append('options', JSON.stringify(options));
 
         const btn = document.getElementById('run-restore-btn');
@@ -304,21 +308,27 @@
         try {
             const res = await fetch('/api/v3/backup/restore', { method: 'POST', body: fd });
             const payload = await res.json();
+            if (payload.status !== 'success') {
+                const msgs = (payload.data?.errors || []).join('; ');
+                throw new Error(payload.message || msgs || 'Restore had errors');
+            }
             const data = payload.data || {};
+            const hasPartial = (data.plugins_failed || []).length > 0 || (data.errors || []).length > 0;
             const result = document.getElementById('restore-result');
-            const ok = payload.status === 'success';
-            result.className = (ok ? 'bg-green-50 border-green-200 text-green-800' : 'bg-yellow-50 border-yellow-200 text-yellow-800') + ' border rounded-md p-4';
+            result.className = (hasPartial
+                ? 'bg-yellow-50 border-yellow-200 text-yellow-800'
+                : 'bg-green-50 border-green-200 text-green-800') + ' border rounded-md p-4';
             result.classList.remove('hidden');
             result.innerHTML = `
-                <h3 class="font-medium mb-2">${ok ? 'Restore complete' : 'Restore finished with warnings'}</h3>
+                <h3 class="font-medium mb-2">${hasPartial ? 'Restore complete with warnings' : 'Restore complete'}</h3>
                 <div><strong>Restored:</strong> ${(data.restored || []).map(escapeHtml).join(', ') || 'none'}</div>
                 <div><strong>Skipped:</strong> ${(data.skipped || []).map(escapeHtml).join(', ') || 'none'}</div>
                 <div><strong>Plugins installed:</strong> ${(data.plugins_installed || []).map(escapeHtml).join(', ') || 'none'}</div>
                 <div><strong>Plugins failed:</strong> ${(data.plugins_failed || []).map(p => escapeHtml(p.plugin_id + ' (' + p.error + ')')).join(', ') || 'none'}</div>
                 <div><strong>Errors:</strong> ${(data.errors || []).map(escapeHtml).join('; ') || 'none'}</div>
-                <p class="mt-2">Restart the display service to apply all changes.</p>
+                ${((data.restored || []).length || (data.plugins_installed || []).length) ? '<p class="mt-2">Restart the display service to apply all changes.</p>' : ''}
             `;
-            notify(ok ? 'Restore complete' : 'Restore finished with warnings', ok ? 'success' : 'info');
+            notify(hasPartial ? 'Restore complete with warnings' : 'Restore complete', hasPartial ? 'warning' : 'success');
         } catch (err) {
             notify('Restore failed: ' + err.message, 'error');
         } finally {
@@ -334,6 +344,13 @@
     window.clearRestore = clearRestore;
     window.runRestore = runRestore;
 
+    // Clear inspection state whenever the user picks a new file.
+    document.getElementById('restore-file-input').addEventListener('change', function () {
+        inspectedFile = null;
+        document.getElementById('restore-preview').classList.add('hidden');
+        document.getElementById('restore-result').classList.add('hidden');
+    });
+
     // Initial load.
     loadPreview();
     loadBackupList();