fix(backup): restore over existing files on hosts without os.chown (#592)

_copy_file() replaces each restored file and then carries the previous
owner across with os.chown. On Windows os.chown does not exist and
st_uid/st_gid are 0 rather than absent, so the ownership branch always
ran and raised AttributeError. That is not an OSError, so it escaped
every per-section handler in restore_backup(): a restore over any
existing config aborted at config.json and restored nothing.

Skip the ownership step where os.chown is missing, as
auto_update_setup.py already does. No change on POSIX.

test_restore_over_a_file_the_user_cannot_write simulates root-owned
files with chmod 0o444; on Windows that sets the read-only attribute,
which blocks any rename over the file, so it is skipped there. The
modes the app writes (0o644/0o640/0o600) replace fine on Windows.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

CHANGELOG.md

@@ -134,6 +134,12 @@ Core:
   from every config load and could not `import web_interface.app` at all.
   `ensure_shared_group_ownership()` now returns immediately when `os.geteuid`
   or `os.chown` is missing. No behaviour change on the Pi.
+- Restoring a backup on Windows no longer fails over files that already exist.
+  The restore carries each replaced file's owner across with `os.chown`, which
+  does not exist on Windows; the `AttributeError` escaped the per-file error
+  handling, so the restore stopped at `config.json` with nothing restored. The
+  ownership step is now skipped where `os.chown` is missing. No behaviour
+  change on the Pi.
 
 ## 3.4.0
 

src/backup_manager.py

@@ -530,12 +530,15 @@ def _copy_file(src: Path, dst: Path) -> None:
             os.chmod(tmp_path, existing_mode)
         else:
             shutil.copymode(src, tmp_path)
-        if existing_owner is not None:
+        if existing_owner is not None and hasattr(os, 'chown'):
             # Replacing a file creates a new inode owned by whoever is running,
             # which would silently move a root-owned config to the web user.
             # Carry the previous owner across when the OS permits it — only
             # root can hand a file to another user, so this is best-effort and
             # a plain restore as the web user simply keeps its own ownership.
+            # os.chown does not exist on Windows (where st_uid/st_gid are just
+            # 0); looking it up there raises AttributeError, which no caller
+            # catches, so every restore over an existing file aborted.
             try:
                 os.chown(tmp_path, existing_owner[0], existing_owner[1])
             except (OSError, PermissionError):

test/test_backup_manager.py

@@ -3,9 +3,12 @@
 from __future__ import annotations
 
 import json
+import os
 import stat
+import sys
 import zipfile
 from pathlib import Path
+from unittest.mock import MagicMock
 
 import pytest
 
@@ -300,6 +303,11 @@ def test_restore_rejects_malicious_zip(empty_project: Path, tmp_path: Path) -> N
     assert any("unsafe" in e.lower() for e in result.errors)
 
 
+@pytest.mark.skipif(
+    sys.platform == "win32",
+    reason="simulates root-owned POSIX files with chmod 0o444, which on Windows sets the "
+    "read-only attribute, and Windows refuses to rename over a read-only file",
+)
 def test_restore_over_a_file_the_user_cannot_write(
     project: Path, empty_project: Path, tmp_path: Path
 ) -> None:
@@ -334,3 +342,53 @@ def test_restore_over_a_file_the_user_cannot_write(
 
     # The destination's mode is preserved rather than widened to the umask.
     assert stat.S_IMODE((empty_project / "config" / "config_secrets.json").stat().st_mode) == 0o444
+
+
+def _existing_config(empty_project: Path) -> None:
+    (empty_project / "config").mkdir(parents=True, exist_ok=True)
+    for name in ("config.json", "config_secrets.json", "wifi_config.json", "ytm_auth.json"):
+        (empty_project / "config" / name).write_text("{}", encoding="utf-8")
+
+
+def test_restore_over_existing_files_without_os_chown(
+    project: Path, empty_project: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Restore must work where the OS has no file ownership API (Windows).
+
+    Replacing a file tries to carry its previous owner across with os.chown.
+    That name does not exist on Windows, and the AttributeError is not an
+    OSError, so it escaped every per-section handler: restoring over any
+    existing config aborted the whole restore and left the old files in place.
+    """
+    zip_path = create_backup(project, output_dir=tmp_path / "exports")
+    _existing_config(empty_project)
+    monkeypatch.delattr(os, "chown", raising=False)
+
+    result = restore_backup(zip_path, empty_project, RestoreOptions())
+
+    assert result.success, result.errors
+    for section in ("config", "secrets", "wifi", "ytm_auth"):
+        assert section in result.restored, f"{section} not restored: {result.errors}"
+    restored = json.loads((empty_project / "config" / "config.json").read_text())
+    assert restored["my-plugin"]["favorites"] == ["A", "B"]
+
+
+def test_restore_still_carries_the_previous_owner_across(
+    project: Path, empty_project: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Where os.chown exists, the replaced file keeps the old file's owner."""
+    zip_path = create_backup(project, output_dir=tmp_path / "exports")
+    _existing_config(empty_project)
+    target = empty_project / "config" / "config.json"
+    old = target.stat()
+    chown = MagicMock()
+    monkeypatch.setattr(os, "chown", chown, raising=False)
+
+    result = restore_backup(zip_path, empty_project, RestoreOptions(
+        restore_secrets=False, restore_wifi=False,
+        restore_fonts=False, restore_plugin_uploads=False, reinstall_plugins=False,
+    ))
+
+    assert result.success, result.errors
+    owners = {(c.args[1], c.args[2]) for c in chown.call_args_list}
+    assert owners == {(old.st_uid, old.st_gid)}