fix(backup): address CodeRabbit and CodeQL findings on PR #439

- first_time_install.sh: verify chown/chmod succeed and the final
  owner/group/mode on config_secrets.json before reporting success;
  exit with a clear error otherwise instead of swallowing failures.
- api_v3.py: replace the predictable .writetest probe with an
  exclusive NamedTemporaryFile to avoid a race with concurrent
  resolvers; log the preferred/fallback export path and OSError when
  falling back to the reinstall-deleted directory.
- api_v3.py: mark a restore as failed when plugin reinstalls fail,
  even if file restoration itself succeeded, so the endpoint no longer
  reports HTTP 200 success on a partial restore.
- api_v3.py: stringify plugin IDs before joining them into the error
  message so a malformed backup's non-string plugin_id can't raise a
  TypeError and mask the detailed response.
- backup_manager.py / api_v3.py: stop putting raw exception text (originating
  from a user-controlled backup file) into restore results returned to
  the client; log full details server-side instead. Addresses the
  CodeQL "stack trace information exposure" alert.
- test coverage: add a test for get_plugin_info() resolving a
  manifest id, and assert the disabled restore_wifi path also skips
  and omits ytm_auth.json.

.github/workflows/test.yml

@@ -86,4 +86,6 @@ jobs:
             test/test_template_targets.py \
             test/test_widget_scripts.py \
             test/test_doc_links.py \
+            test/test_registry_id_resolution.py \
+            test/test_backup_manager.py \
             test/web_interface/test_cache.py

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
-    chmod 640 "$PROJECT_ROOT_DIR/config/config_secrets.json"
+    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
+    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 json
 import logging
 import os
 import shutil
+import stat
 import socket
 import tempfile
 import zipfile
@@ -82,6 +83,10 @@ BUNDLED_FONTS: frozenset[str] = frozenset({
 _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 @@ def create_backup(
             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 @@ def preview_backup_contents(project_root: Path) -> Dict[str, Any]:
         "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 @@ def validate_backup(zip_path: Path) -> Tuple[bool, str, Dict[str, Any]]:
                 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 _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(
@@ -513,7 +577,8 @@ def restore_backup(
         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 @@ def restore_backup(
                 _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 @@ def restore_backup(
                 _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 @@ def restore_backup(
                 _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 @@ def restore_backup(
                     _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 @@ def restore_backup(
                         _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 @@ def restore_backup(
                         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 @@ class PluginStoreManager:
         """
         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 @@ class PluginStoreManager:
 
         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 @@ class PluginStoreManager:
         """
         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

test/test_backup_manager.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import json
+import stat
 import zipfile
 from pathlib import Path
 
@@ -41,6 +42,13 @@ def _make_project(root: Path) -> Path:
         json.dumps({"ap_mode": {"ssid": "LEDMatrix"}}),
         encoding="utf-8",
     )
+    # Device-local auth that lives in config/ like the three above. It was
+    # omitted from backups, so a restore silently signed the user out of
+    # YouTube Music and they had to re-authenticate by hand.
+    (root / "config" / "ytm_auth.json").write_text(
+        json.dumps({"token": "YTM-TOKEN"}),
+        encoding="utf-8",
+    )
 
     fonts = root / "assets" / "fonts"
     fonts.mkdir(parents=True)
@@ -240,6 +248,10 @@ def test_restore_roundtrip(project: Path, empty_project: Path, tmp_path: Path) -
     restored_secrets = json.loads((empty_project / "config" / "config_secrets.json").read_text())
     assert restored_secrets["ledmatrix-weather"]["api_key"] == "SECRET"
 
+    assert "ytm_auth" in result.restored
+    restored_ytm = json.loads((empty_project / "config" / "ytm_auth.json").read_text())
+    assert restored_ytm["token"] == "YTM-TOKEN"
+
     # User font restored, bundled font untouched.
     assert (empty_project / "assets" / "fonts" / "my-custom-font.ttf").read_bytes() == b"\x00\x01USER"
     assert (empty_project / "assets" / "fonts" / "5x7.bdf").read_text() == "BUNDLED"
@@ -271,6 +283,10 @@ def test_restore_honors_options(project: Path, empty_project: Path, tmp_path: Pa
     assert result.plugins_to_install == []
     assert "secrets" in result.skipped
     assert "wifi" in result.skipped
+    # ytm_auth rides on restore_wifi rather than its own flag -- disabling
+    # wifi restore must not leave a stale session token behind.
+    assert "ytm_auth" in result.skipped
+    assert not (empty_project / "config" / "ytm_auth.json").exists()
 
 
 def test_restore_rejects_malicious_zip(empty_project: Path, tmp_path: Path) -> None:
@@ -282,3 +298,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

test/test_registry_id_resolution.py

@@ -0,0 +1,114 @@
+"""A plugin must be findable in the registry by the id it calls itself.
+
+Four shipped plugins have a registry ``id`` that differs from the ``id`` in
+their own ``manifest.json``:
+
+    directory / manifest.json id      registry id
+    ledmatrix-weather                 weather
+    ledmatrix-stocks                  stocks
+    ledmatrix-music                   music
+    ledmatrix-leaderboard             leaderboard
+
+The installer already knows about this: it deliberately names the install
+directory after the *manifest* id (store_manager, "Use manifest ID for
+directory name"), and warns when the two disagree. So on disk, in
+``config.json`` and in a backup manifest, these plugins are called
+``ledmatrix-weather``. Only the registry calls them ``weather``.
+
+Nothing resolved that in reverse. Asking the store to install
+``ledmatrix-weather`` -- which is exactly what restoring a backup does --
+failed with "Plugin not found in registry", and four enabled plugins went
+missing from a restored device with no error surfaced to the user.
+
+Renaming the registry ids would orphan existing ``plugin_state.json`` entries
+keyed on the old ones, so the lookup resolves ``plugin_path`` instead: the
+registry already records ``plugins/ledmatrix-weather``, which is unambiguous
+and needs no published identity to change.
+"""
+
+import os
+import sys
+from typing import Any, Dict, List, Optional
+
+import pytest
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
+
+from src.plugin_system.store_manager import PluginStoreManager  # noqa: E402
+
+
+# Shaped like the real registry: id and plugin_path basename disagree for the
+# first entry, agree for the second.
+REGISTRY: Dict[str, List[Dict[str, Any]]] = {
+    "plugins": [
+        {
+            "id": "weather",
+            "name": "Weather",
+            "plugin_path": "plugins/ledmatrix-weather",
+            "repo": "https://github.com/ChuckBuilds/ledmatrix-plugins",
+        },
+        {
+            "id": "ledmatrix-flights",
+            "name": "Flights",
+            "plugin_path": "plugins/ledmatrix-flights",
+            "repo": "https://github.com/ChuckBuilds/ledmatrix-plugins",
+        },
+        {
+            "id": "third-party",
+            "name": "Third Party",
+            "plugin_path": "",
+            "repo": "https://github.com/someone/thing",
+        },
+    ]
+}
+
+
+@pytest.fixture
+def store(monkeypatch: pytest.MonkeyPatch) -> PluginStoreManager:
+    manager = PluginStoreManager.__new__(PluginStoreManager)
+    monkeypatch.setattr(manager, "fetch_registry", lambda *a, **k: REGISTRY, raising=False)
+    return manager
+
+
+def _ids(entry: Optional[Dict[str, Any]]) -> Optional[str]:
+    return entry.get("id") if entry else None
+
+
+class TestRegistryLookupByManifestId:
+    def test_get_plugin_info_resolves_manifest_id(self, store: PluginStoreManager) -> None:
+        """get_plugin_info() delegates to the same lookup as get_registry_info()."""
+        assert (
+            _ids(store.get_plugin_info("ledmatrix-weather", fetch_latest_from_github=False))
+            == "weather"
+        )
+
+    def test_exact_registry_id_still_resolves(self, store: PluginStoreManager) -> None:
+        assert _ids(store.get_registry_info("weather")) == "weather"
+
+    def test_manifest_id_resolves_via_plugin_path(self, store: PluginStoreManager) -> None:
+        """The case that broke restore: asked by the name on disk."""
+        assert _ids(store.get_registry_info("ledmatrix-weather")) == "weather", (
+            "a plugin installed as 'ledmatrix-weather' could not be found in a "
+            "registry that lists it under plugin_path plugins/ledmatrix-weather")
+
+    def test_matching_id_and_path_unaffected(self, store: PluginStoreManager) -> None:
+        assert _ids(store.get_registry_info("ledmatrix-flights")) == "ledmatrix-flights"
+
+    def test_unknown_plugin_still_returns_none(self, store: PluginStoreManager) -> None:
+        assert store.get_registry_info("no-such-plugin") is None
+
+    def test_empty_plugin_path_is_not_a_wildcard(self, store: PluginStoreManager) -> None:
+        """Third-party entries carry plugin_path "" — that must not match ""."""
+        assert store.get_registry_info("") is None
+
+    def test_exact_id_wins_over_a_path_match(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        """If some other entry's path collides with a real id, id wins."""
+        registry = {
+            "plugins": [
+                {"id": "decoy", "plugin_path": "plugins/weather"},
+                {"id": "weather", "plugin_path": "plugins/ledmatrix-weather"},
+            ]
+        }
+        manager = PluginStoreManager.__new__(PluginStoreManager)
+        monkeypatch.setattr(manager, "fetch_registry", lambda *a, **k: registry, raising=False)
+        assert _ids(manager.get_registry_info("weather")) == "weather"

web_interface/blueprints/api_v3.py

@@ -7848,7 +7848,35 @@ def clear_old_errors():
 # Backup / Restore
 # ---------------------------------------------------------------------------
 
-_BACKUP_EXPORT_DIR = PROJECT_ROOT / "config" / "backups" / "exports"
+def _resolve_backup_export_dir() -> Path:
+    """Where exported backups live: beside the install, not inside it.
+
+    They used to be written to ``<project>/config/backups/exports``. That is
+    inside the directory a reinstall deletes, so the documented recovery path
+    -- export a backup, then reinstall -- destroyed the backup it had just
+    told the user to make. Anyone who downloaded the ZIP was fine; anyone
+    relying on the on-device copy was not.
+
+    Falls back to the old location when the parent directory is not writable,
+    so an unusual layout degrades to previous behaviour instead of failing to
+    export at all.
+    """
+    preferred = PROJECT_ROOT.parent / "ledmatrix-backups"
+    fallback = PROJECT_ROOT / "config" / "backups" / "exports"
+    try:
+        preferred.mkdir(parents=True, exist_ok=True)
+        with tempfile.NamedTemporaryFile(dir=preferred, prefix=".writetest-"):
+            pass
+        return preferred
+    except OSError as e:
+        logger.warning(
+            f"[Backup] Export dir {preferred} is not writable ({e}); "
+            f"falling back to {fallback}, which a reinstall will delete"
+        )
+        return fallback
+
+
+_BACKUP_EXPORT_DIR = _resolve_backup_export_dir()
 
 
 def _safe_backup_path(filename: str) -> Path:
@@ -7998,11 +8026,36 @@ def backup_restore():
                     else:
                         result.plugins_failed.append({'plugin_id': pid, 'error': 'Store manager unavailable'})
                 except Exception as pe:
-                    result.plugins_failed.append({'plugin_id': pid, 'error': str(pe)})
+                    logger.error(
+                        "[Backup] Failed to reinstall plugin %r: %s", pid, pe, exc_info=True
+                    )
+                    result.plugins_failed.append({'plugin_id': pid, 'error': 'Installation failed; see server logs'})
+
+        # A restore that dropped files can still report success if the only
+        # failures were plugin reinstalls, since those don't touch result.errors.
+        if result.plugins_failed:
+            result.success = False
 
         data = result.to_dict()
         if not result.success:
-            return jsonify({'status': 'error', 'message': 'Restore had errors', 'data': data}), 500
+            # Name what failed, and what nonetheless landed. A restore is
+            # partial far more often than it is total -- a fresh install can
+            # leave config_secrets.json unwritable by the web service, so
+            # config restores and secrets do not. "Restore had errors" alone
+            # left the user unable to tell a wholly failed restore from one
+            # that quietly dropped their API keys.
+            failed_plugins = [
+                str(p.get('plugin_id')) for p in (result.plugins_failed or []) if p.get('plugin_id')
+            ]
+            parts = []
+            if result.restored:
+                parts.append(f"restored: {', '.join(result.restored)}")
+            if result.errors:
+                parts.append(f"failed: {'; '.join(result.errors)}")
+            if failed_plugins:
+                parts.append(f"plugins not reinstalled: {', '.join(failed_plugins)}")
+            message = 'Restore incomplete — ' + ('. '.join(parts) if parts else 'see logs')
+            return jsonify({'status': 'error', 'message': message, 'data': data}), 500
         return jsonify({'status': 'success', 'data': data})
     except Exception as e:
         logger.error("backup_restore failed: %s", e, exc_info=True)