fix(backup): restore must not depend on owning the file it replaces

Follow-up from testing the previous commit on real hardware, where the
secrets fix turned out to be both too narrow and slightly wrong.

Too narrow: config.json, wifi_config.json and ytm_auth.json are installed
root-owned and group-readable exactly like the secrets file, so all four
were unrestorable by the web service, not just one. `shutil.copy2` opens
the destination for writing, which needs permission on the *existing
file*; the web user could create files in that directory all day and
still not replace them.

Slightly wrong: the previous commit loosened the secrets file to
group-writable. That was treating the symptom. The real error was
deciding ownership from `ledmatrix.service` -- the display service, which
runs as root and only ever *reads* secrets -- when the account that
*writes* them is the web interface, which deliberately does not run as
root. Ownership now follows the web service's user and the mode stays
640.

`_copy_file` writes a temporary file alongside the target and renames
over it. That needs only directory permission, so a restore no longer
cares who owns the destination, and it is atomic: a crash mid-restore can
no longer leave a half-written config. The destination's mode is carried
across so restoring secrets does not widen them to the umask, and its
owner is carried across too when the OS allows it -- only root can hand a
file to another user, so a restore run by the web service keeps its own
ownership rather than pretending to preserve root's.

Verified on a device with all four config files set root-owned 640 and
unwritable by the web user: before, every one failed with EACCES; after,
the restore reports success with no errors and all four sections
restored, mode still 640, root still able to read them and the web
service still able to write them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

first_time_install.sh

@@ -1483,34 +1483,38 @@ if [ -f "$PROJECT_ROOT_DIR/config/config.json" ]; then
 fi
 
 # Set proper permissions for secrets file (restrictive: owner rw, group r)
-# If service runs as root, set ownership to root so it can read as owner
-# Otherwise, use ACTUAL_USER and rely on group membership
+# Owned by whoever WRITES the file, which is the web interface.
+#
+# This used to read the User= of ledmatrix.service — the display service —
+# and, finding root, hand the file to root:ledmatrix 640. But the display
+# service only ever reads secrets, and root can read any file regardless of
+# mode. The account that *writes* them is the web interface: it saves config
+# edits and performs backup restores, and it deliberately does not run as root
+# (a web server should not). So a root-owned, group-read-only file left the web
+# UI unable to write its own secrets, and restoring a backup failed with
+# "Permission denied: config_secrets.json" while every other file in the same
+# backup restored fine.
+#
+# Owning by the writer keeps the tighter 640 rather than loosening to
+# group-writable, and root still reads it as superuser.
 if [ -f "$PROJECT_ROOT_DIR/config/config_secrets.json" ]; then
-    # Check if service runs as root (from service file or template)
-    SERVICE_USER="root"
-    if [ -f "/etc/systemd/system/ledmatrix.service" ]; then
-        SERVICE_USER=$(grep "^User=" /etc/systemd/system/ledmatrix.service | cut -d'=' -f2 || echo "root")
-    elif [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix.service" ]; then
-        SERVICE_USER=$(grep "^User=" "$PROJECT_ROOT_DIR/systemd/ledmatrix.service" | cut -d'=' -f2 || echo "root")
+    # The web service is the writer; fall back to the display service, then to
+    # the installing user, so an unusual layout still lands somewhere sensible.
+    SECRETS_OWNER=""
+    for unit in "/etc/systemd/system/ledmatrix-web.service" \
+                "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service"; do
+        if [ -f "$unit" ]; then
+            SECRETS_OWNER=$(grep -m1 "^User=" "$unit" | cut -d'=' -f2)
+            [ -n "$SECRETS_OWNER" ] && break
         fi
-    
-    if [ "$SERVICE_USER" = "root" ]; then
-        # Service runs as root - set ownership to root so it can read as owner
-        chown "root:$LEDMATRIX_GROUP" "$PROJECT_ROOT_DIR/config/config_secrets.json" || true
-        echo "✓ Secrets file permissions set (root:$LEDMATRIX_GROUP for root service)"
-    else
-        # Service runs as regular user - use ACTUAL_USER and rely on group membership
-        chown "$ACTUAL_USER:$LEDMATRIX_GROUP" "$PROJECT_ROOT_DIR/config/config_secrets.json" || true
-        echo "✓ Secrets file permissions set ($ACTUAL_USER:$LEDMATRIX_GROUP)"
+    done
+    if [ -z "$SECRETS_OWNER" ]; then
+        SECRETS_OWNER="$ACTUAL_USER"
     fi
-    # Group-writable, not just group-readable. The web interface performs
-    # backup restores, and it does not necessarily run as the file's owner: on
-    # a fresh install the secrets file ended up owned by root while the web
-    # service ran as the login user, so restoring a backup failed with
-    # "Permission denied: config_secrets.json" while every other file in the
-    # same backup restored fine. Read-only for the group makes secrets the one
-    # thing a restore cannot put back.
-    chmod 660 "$PROJECT_ROOT_DIR/config/config_secrets.json"
+    # A root-owned file is only correct when the writer really is root.
+    chown "$SECRETS_OWNER:$LEDMATRIX_GROUP" "$PROJECT_ROOT_DIR/config/config_secrets.json" || true
+    echo "✓ Secrets file owned by the web service user ($SECRETS_OWNER:$LEDMATRIX_GROUP, mode 640)"
+    chmod 640 "$PROJECT_ROOT_DIR/config/config_secrets.json"
 fi
 
 # Set proper permissions for YTM auth file (readable by all users including root service)

src/backup_manager.py

@@ -16,6 +16,7 @@ import json
 import logging
 import os
 import shutil
+import stat
 import socket
 import tempfile
 import zipfile
@@ -491,8 +492,61 @@ def _extract_zip_safe(zip_path: Path, dest_dir: Path) -> None:
 
 
 def _copy_file(src: Path, dst: Path) -> None:
+    """Replace ``dst`` with ``src``, atomically, without needing to own ``dst``.
+
+    ``shutil.copy2`` opens the destination for writing, so it needs write
+    permission on the *existing file*. Several config files are installed
+    root-owned and group-readable while the web interface — which is what runs
+    a restore — deliberately runs as a non-root user. Restoring those failed
+    with EACCES even though the account could create files in the same
+    directory perfectly well.
+
+    Writing a temporary file alongside and renaming over the target needs only
+    directory permission, which the web user has. It is also atomic: a crash
+    mid-restore can no longer leave a half-written config behind.
+
+    The destination's existing mode is preserved when there is one, so
+    restoring secrets does not silently widen them to the umask default.
+    """
     dst.parent.mkdir(parents=True, exist_ok=True)
-    shutil.copy2(src, dst)
+
+    existing_mode: Optional[int] = None
+    existing_owner: Optional[Tuple[int, int]] = None
+    if dst.exists():
+        try:
+            info = dst.stat()
+            existing_mode = stat.S_IMODE(info.st_mode)
+            existing_owner = (info.st_uid, info.st_gid)
+        except OSError:
+            existing_mode = None
+            existing_owner = None
+
+    fd, tmp_name = tempfile.mkstemp(dir=str(dst.parent), prefix=f".{dst.name}.", suffix=".tmp")
+    os.close(fd)
+    tmp_path = Path(tmp_name)
+    try:
+        shutil.copyfile(src, tmp_path)
+        if existing_mode is not None:
+            os.chmod(tmp_path, existing_mode)
+        else:
+            shutil.copymode(src, tmp_path)
+        if existing_owner is not None:
+            # 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.
+            try:
+                os.chown(tmp_path, existing_owner[0], existing_owner[1])
+            except (OSError, PermissionError):
+                pass
+        os.replace(tmp_path, dst)
+    except BaseException:
+        try:
+            tmp_path.unlink()
+        except OSError:
+            pass
+        raise
 
 
 def restore_backup(

test/test_backup_manager.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import json
+import stat
 import zipfile
 from pathlib import Path
 
@@ -293,3 +294,39 @@ def test_restore_rejects_malicious_zip(empty_project: Path, tmp_path: Path) -> N
     # validate_backup catches it before extraction.
     assert not result.success
     assert any("unsafe" in e.lower() for e in result.errors)
+
+
+def test_restore_over_a_file_the_user_cannot_write(
+    project: Path, empty_project: Path, tmp_path: Path
+) -> None:
+    """Restore must not need write permission on the destination *file*.
+
+    Reproduces what a fresh install leaves behind: config files owned by root
+    and only group-readable, while the web interface that performs the restore
+    runs as a non-root user. shutil.copy2 opens the destination for writing and
+    failed with EACCES; writing alongside and renaming needs only directory
+    permission, which that account has.
+
+    Simulated here by making the destination read-only — the owner cannot
+    open it for writing either, but can still replace it within its directory.
+    """
+    zip_path = create_backup(project, output_dir=tmp_path / "exports")
+
+    # Pre-existing, read-only destinations.
+    (empty_project / "config").mkdir(parents=True, exist_ok=True)
+    for name in ("config.json", "config_secrets.json", "wifi_config.json", "ytm_auth.json"):
+        target = empty_project / "config" / name
+        target.write_text("{}", encoding="utf-8")
+        target.chmod(0o444)
+
+    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"]
+
+    # 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