diff --git a/README.md b/README.md index 0d50c38e..b901dde3 100644 --- a/README.md +++ b/README.md @@ -357,6 +357,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 +``` + diff --git a/first_time_install.sh b/first_time_install.sh index d8ef6338..1eaf2d68 100644 --- a/first_time_install.sh +++ b/first_time_install.sh @@ -1497,13 +1497,20 @@ if [ -f "$PROJECT_ROOT_DIR/config/config_secrets.json" ]; then 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)" + 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)" + echo "✓ Secrets file permissions set ($ACTUAL_USER:$LEDMATRIX_GROUP)" fi - chmod 640 "$PROJECT_ROOT_DIR/config/config_secrets.json" + # 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" fi # Set proper permissions for YTM auth file (readable by all users including root service) diff --git a/src/backup_manager.py b/src/backup_manager.py index c54ce81e..af3247b6 100644 --- a/src/backup_manager.py +++ b/src/backup_manager.py @@ -82,6 +82,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 +307,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 +355,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 +437,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( @@ -546,6 +556,18 @@ def restore_backup( 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: + result.errors.append(f"Failed to restore ytm_auth.json: {e}") + 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(): diff --git a/src/plugin_system/store_manager.py b/src/plugin_system/store_manager.py index e13b3e5b..8e4819e3 100644 --- a/src/plugin_system/store_manager.py +++ b/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 diff --git a/test/test_backup_manager.py b/test/test_backup_manager.py index fef10d82..97c4d000 100644 --- a/test/test_backup_manager.py +++ b/test/test_backup_manager.py @@ -41,6 +41,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 +247,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" diff --git a/test/test_registry_id_resolution.py b/test/test_registry_id_resolution.py new file mode 100644 index 00000000..85fb0084 --- /dev/null +++ b/test/test_registry_id_resolution.py @@ -0,0 +1,107 @@ +"""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_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" diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index 9b4fec57..c7ed438d 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -7843,7 +7843,31 @@ 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 ``/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" + try: + preferred.mkdir(parents=True, exist_ok=True) + probe = preferred / ".writetest" + probe.write_text("", encoding="utf-8") + probe.unlink() + return preferred + except OSError: + return PROJECT_ROOT / "config" / "backups" / "exports" + + +_BACKUP_EXPORT_DIR = _resolve_backup_export_dir() def _safe_backup_path(filename: str) -> Path: @@ -7997,7 +8021,22 @@ def backup_restore(): 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 = [p.get('plugin_id') for p in (result.plugins_failed or [])] + 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)