fix(backup): make a restored device match the one that was backed up

README.md

@@ -365,6 +365,12 @@ sudo bash ./first_time_install.sh
 
 This single script installs services, dependencies, configures permissions and sudoers, and validates the setup.
 
+It finishes by asking whether to reboot. If you run it non-interactively — piped, over a script, or with `-y` — there is no one to ask, so **it reboots immediately without prompting**. Pass `--no-reboot-prompt` to install without rebooting:
+
+```bash
+sudo bash ./first_time_install.sh -y --no-reboot-prompt
+```
+
 </details>
 
 </details>

first_time_install.sh

@@ -1480,27 +1480,54 @@ 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
+    done
+    if [ -z "$SECRETS_OWNER" ]; then
+        SECRETS_OWNER="$ACTUAL_USER"
     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 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)"
+    SECRETS_FILE="$PROJECT_ROOT_DIR/config/config_secrets.json"
+    # A root-owned file is only correct when the writer really is root.
+    if ! chown "$SECRETS_OWNER:$LEDMATRIX_GROUP" "$SECRETS_FILE"; then
+        echo "✗ ERROR: Failed to set ownership on $SECRETS_FILE to $SECRETS_OWNER:$LEDMATRIX_GROUP" >&2
+        echo "  Try: sudo chown $SECRETS_OWNER:$LEDMATRIX_GROUP $SECRETS_FILE" >&2
+        exit 1
+    fi
+    if ! chmod 640 "$SECRETS_FILE"; then
+        echo "✗ ERROR: Failed to set permissions on $SECRETS_FILE to 640" >&2
+        echo "  Try: sudo chmod 640 $SECRETS_FILE" >&2
+        exit 1
+    fi
+    ACTUAL_OWNERSHIP=$(stat -c '%U:%G' "$SECRETS_FILE" 2>/dev/null || echo "unknown")
+    ACTUAL_MODE=$(stat -c '%a' "$SECRETS_FILE" 2>/dev/null || echo "unknown")
+    if [ "$ACTUAL_OWNERSHIP" != "$SECRETS_OWNER:$LEDMATRIX_GROUP" ] || [ "$ACTUAL_MODE" != "640" ]; then
+        echo "✗ ERROR: $SECRETS_FILE ended up as $ACTUAL_OWNERSHIP mode $ACTUAL_MODE, expected $SECRETS_OWNER:$LEDMATRIX_GROUP mode 640" >&2
+        echo "  The web interface may be unable to read or write config_secrets.json." >&2
+        exit 1
     fi
-    chmod 640 "$PROJECT_ROOT_DIR/config/config_secrets.json"
+    echo "✓ Secrets file owned by the web service user ($SECRETS_OWNER:$LEDMATRIX_GROUP, mode 640)"
 fi
 
 # Set proper permissions for YTM auth file (readable by all users including root service)

src/backup_manager.py

@@ -16,6 +16,7 @@
 import logging
 import os
 import shutil
+import stat
 import socket
 import tempfile
 import zipfile
@@ -82,6 +83,10 @@
 _CONFIG_REL = Path("config/config.json")
 _SECRETS_REL = Path("config/config_secrets.json")
 _WIFI_REL = Path("config/wifi_config.json")
+# Sits in config/ next to the three above and is pure user state — a
+# YouTube Music session that has to be re-authenticated by hand if lost.
+# It was omitted from backups, so a restore silently signed the user out.
+_YTM_REL = Path("config/ytm_auth.json")
 _FONTS_REL = Path("assets/fonts")
 _PLUGIN_UPLOADS_REL = Path("assets/plugins")
 _STATE_REL = Path("data/plugin_state.json")
@@ -303,6 +308,9 @@
             if (project_root / _WIFI_REL).exists():
                 zf.write(project_root / _WIFI_REL, _WIFI_REL.as_posix())
                 contents.append("wifi")
+            if (project_root / _YTM_REL).exists():
+                zf.write(project_root / _YTM_REL, _YTM_REL.as_posix())
+                contents.append("ytm_auth")
 
             # User-uploaded fonts.
             user_fonts = iter_user_fonts(project_root)
@@ -348,6 +356,7 @@
         "has_config": (project_root / _CONFIG_REL).exists(),
         "has_secrets": (project_root / _SECRETS_REL).exists(),
         "has_wifi": (project_root / _WIFI_REL).exists(),
+        "has_ytm_auth": (project_root / _YTM_REL).exists(),
         "user_fonts": [p.name for p in iter_user_fonts(project_root)],
         "plugin_uploads": len(iter_plugin_uploads(project_root)),
         "plugins": list_installed_plugins(project_root),
@@ -429,6 +438,8 @@
                 detected.append("secrets")
             if _WIFI_REL.as_posix() in names:
                 detected.append("wifi")
+            if _YTM_REL.as_posix() in names:
+                detected.append("ytm_auth")
             if any(n.startswith(_FONTS_REL.as_posix() + "/") for n in names):
                 detected.append("fonts")
             if any(
@@ -481,8 +492,61 @@
 
 
 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(
@@ -513,7 +577,8 @@
         try:
             _extract_zip_safe(Path(zip_path), tmp_dir)
         except (ValueError, zipfile.BadZipFile, OSError) as e:
-            result.errors.append(f"Failed to extract backup: {e}")
+            logger.error("[Backup] Failed to extract backup: %s", e, exc_info=True)
+            result.errors.append("Failed to extract backup")
             return result
 
         # Main config.
@@ -522,7 +587,8 @@
                 _copy_file(tmp_dir / _CONFIG_REL, project_root / _CONFIG_REL)
                 result.restored.append("config")
             except OSError as e:
-                result.errors.append(f"Failed to restore config.json: {e}")
+                logger.error("[Backup] Failed to restore config.json: %s", e, exc_info=True)
+                result.errors.append("Failed to restore config.json")
         elif (tmp_dir / _CONFIG_REL).exists():
             result.skipped.append("config")
 
@@ -532,7 +598,10 @@
                 _copy_file(tmp_dir / _SECRETS_REL, project_root / _SECRETS_REL)
                 result.restored.append("secrets")
             except OSError as e:
-                result.errors.append(f"Failed to restore config_secrets.json: {e}")
+                logger.error(
+                    "[Backup] Failed to restore config_secrets.json: %s", e, exc_info=True
+                )
+                result.errors.append("Failed to restore config_secrets.json")
         elif (tmp_dir / _SECRETS_REL).exists():
             result.skipped.append("secrets")
 
@@ -542,10 +611,26 @@
                 _copy_file(tmp_dir / _WIFI_REL, project_root / _WIFI_REL)
                 result.restored.append("wifi")
             except OSError as e:
-                result.errors.append(f"Failed to restore wifi_config.json: {e}")
+                logger.error(
+                    "[Backup] Failed to restore wifi_config.json: %s", e, exc_info=True
+                )
+                result.errors.append("Failed to restore wifi_config.json")
         elif (tmp_dir / _WIFI_REL).exists():
             result.skipped.append("wifi")
 
+        # YouTube Music session. Follows restore_wifi rather than getting its
+        # own flag: it is device-local auth in the same sense, and a separate
+        # toggle for one file would be noise in the restore dialog.
+        if options.restore_wifi and (tmp_dir / _YTM_REL).exists():
+            try:
+                _copy_file(tmp_dir / _YTM_REL, project_root / _YTM_REL)
+                result.restored.append("ytm_auth")
+            except OSError as e:
+                logger.error("[Backup] Failed to restore ytm_auth.json: %s", e, exc_info=True)
+                result.errors.append("Failed to restore ytm_auth.json")
+        elif (tmp_dir / _YTM_REL).exists():
+            result.skipped.append("ytm_auth")
+
         # User fonts — skip anything that collides with a bundled font.
         tmp_fonts = tmp_dir / _FONTS_REL
         if options.restore_fonts and tmp_fonts.exists():
@@ -560,7 +645,10 @@
                     _copy_file(font, project_root / _FONTS_REL / font.name)
                     restored_count += 1
                 except OSError as e:
-                    result.errors.append(f"Failed to restore font {font.name}: {e}")
+                    logger.error(
+                        "[Backup] Failed to restore font %s: %s", font.name, e, exc_info=True
+                    )
+                    result.errors.append(f"Failed to restore font {font.name}")
             if restored_count:
                 result.restored.append(f"fonts ({restored_count})")
         elif tmp_fonts.exists():
@@ -581,7 +669,8 @@
                         _copy_file(src, project_root / rel)
                         count += 1
                     except OSError as e:
-                        result.errors.append(f"Failed to restore {rel}: {e}")
+                        logger.error("[Backup] Failed to restore %s: %s", rel, e, exc_info=True)
+                        result.errors.append(f"Failed to restore {rel}")
             if count:
                 result.restored.append(f"plugin_uploads ({count})")
         elif tmp_uploads.exists():
@@ -599,7 +688,8 @@
                         if isinstance(p, dict) and p.get("plugin_id")
                     ]
             except (OSError, json.JSONDecodeError) as e:
-                result.errors.append(f"Could not read plugins.json: {e}")
+                logger.error("[Backup] Could not read plugins.json: %s", e, exc_info=True)
+                result.errors.append("Could not read plugins.json")
 
     result.success = not result.errors
     return result

src/plugin_system/store_manager.py

@@ -1143,7 +1143,7 @@ def get_plugin_info(self, plugin_id: str, fetch_latest_from_github: bool = True,
         """
         registry = self.fetch_registry()
         plugins = registry.get('plugins', []) or []
-        plugin_info = next((p for p in plugins if p['id'] == plugin_id), None)
+        plugin_info = self._match_registry_entry(plugins, plugin_id)
 
         if not plugin_info:
             return None
@@ -1183,6 +1183,37 @@ def get_plugin_info(self, plugin_id: str, fetch_latest_from_github: bool = True,
 
         return plugin_info
 
+    @staticmethod
+    def _match_registry_entry(plugins: List[Dict], plugin_id: str) -> Optional[Dict]:
+        """Find a registry entry by its id, or by the directory it installs to.
+
+        Four shipped plugins have a registry ``id`` that differs from the ``id``
+        in their own manifest: ``weather`` installs to ``plugins/ledmatrix-weather``,
+        and likewise stocks, music and leaderboard. Installation already prefers
+        the manifest id for the directory name, so on disk, in ``config.json``
+        and in a backup manifest those plugins are called ``ledmatrix-weather``.
+
+        Only the registry calls them ``weather``, and nothing resolved that in
+        reverse: restoring a backup asked the store for ``ledmatrix-weather``
+        and got "Plugin not found in registry", silently dropping four enabled
+        plugins from a restored device.
+
+        Matching ``plugin_path`` fixes it without renaming any published id,
+        which would orphan ``plugin_state.json`` entries keyed on the old ones.
+        Exact id always wins, so an entry whose *path* happens to collide with
+        another entry's id cannot shadow it.
+        """
+        if not plugin_id:
+            return None
+        exact = next((p for p in plugins if p.get('id') == plugin_id), None)
+        if exact is not None:
+            return exact
+        for entry in plugins:
+            path = (entry.get('plugin_path') or '').rstrip('/')
+            if path and path.rsplit('/', 1)[-1] == plugin_id:
+                return entry
+        return None
+
     def get_registry_info(self, plugin_id: str) -> Optional[Dict]:
         """
         Get plugin information from the registry cache only (no GitHub API calls).
@@ -1198,7 +1229,7 @@ def get_registry_info(self, plugin_id: str) -> Optional[Dict]:
         """
         registry = self.fetch_registry()
         plugins = registry.get('plugins', []) or []
-        return next((p for p in plugins if p.get('id') == plugin_id), None)
+        return self._match_registry_entry(plugins, plugin_id)
 
     def install_plugin(self, plugin_id: str, branch: Optional[str] = None) -> bool:
         """Install a plugin, keeping any existing install until the new one is