mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-07 11:48:06 +00:00
Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7d83ca742a | ||
|
|
d44e31ac9d | ||
|
|
5cf92f29b1 |
@@ -85,6 +85,7 @@ jobs:
|
|||||||
test/test_plugin_update_reservation.py \
|
test/test_plugin_update_reservation.py \
|
||||||
test/test_template_targets.py \
|
test/test_template_targets.py \
|
||||||
test/test_widget_scripts.py \
|
test/test_widget_scripts.py \
|
||||||
test/test_enum_option_labels.py \
|
|
||||||
test/test_doc_links.py \
|
test/test_doc_links.py \
|
||||||
|
test/test_registry_id_resolution.py \
|
||||||
|
test/test_backup_manager.py \
|
||||||
test/web_interface/test_cache.py
|
test/web_interface/test_cache.py
|
||||||
|
|||||||
@@ -365,6 +365,12 @@ sudo bash ./first_time_install.sh
|
|||||||
|
|
||||||
This single script installs services, dependencies, configures permissions and sudoers, and validates the setup.
|
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>
|
||||||
|
|
||||||
</details>
|
</details>
|
||||||
|
|||||||
@@ -206,45 +206,6 @@ To use an existing widget in your plugin's `config_schema.json`, simply add the
|
|||||||
|
|
||||||
The widget will be automatically rendered when the plugin configuration form is loaded.
|
The widget will be automatically rendered when the plugin configuration form is loaded.
|
||||||
|
|
||||||
## Labelling Enum Options (`x-options.labels`)
|
|
||||||
|
|
||||||
A plain `enum` renders as a dropdown whose option text is the value with
|
|
||||||
underscores replaced and title case applied — `day_first` becomes "Day First".
|
|
||||||
That is fine for values that read as their own label, and wrong for values that
|
|
||||||
do not: `vs` becomes "Vs", and `abbrev` says nothing about the `Sep 19` it
|
|
||||||
actually produces.
|
|
||||||
|
|
||||||
Supply `x-options.labels` to set the visible text. This is the same convention
|
|
||||||
the `checkbox-group` widget uses:
|
|
||||||
|
|
||||||
```json
|
|
||||||
{
|
|
||||||
"date_format": {
|
|
||||||
"type": "string",
|
|
||||||
"enum": ["abbrev", "numeric", "day_first"],
|
|
||||||
"default": "abbrev",
|
|
||||||
"x-options": {
|
|
||||||
"labels": {
|
|
||||||
"abbrev": "Sep 19",
|
|
||||||
"numeric": "9/19",
|
|
||||||
"day_first": "19 Sep"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
Labels are **display only** — the stored value is still the enum value, so
|
|
||||||
adding them never changes a saved config. The map may be partial: any value
|
|
||||||
without a label keeps the humanised fallback. Older cores that predate this
|
|
||||||
support ignore `x-options` and render the fallback for every option, so a
|
|
||||||
plugin can ship labels without requiring a core upgrade.
|
|
||||||
|
|
||||||
Array-table columns (`x-widget: array-table`) accept the same
|
|
||||||
`x-options.labels` on a column definition, but their fallback is the **raw
|
|
||||||
value** rather than the humanised one, because those columns hold values such
|
|
||||||
as ticker symbols where `aapl` → "Aapl" would be wrong.
|
|
||||||
|
|
||||||
## Marking Fields as Advanced (`x-advanced`)
|
## Marking Fields as Advanced (`x-advanced`)
|
||||||
|
|
||||||
Add `"x-advanced": true` to any top-level, non-object property to move it out
|
Add `"x-advanced": true` to any top-level, non-object property to move it out
|
||||||
|
|||||||
+28
-17
@@ -1480,26 +1480,37 @@ if [ -f "$PROJECT_ROOT_DIR/config/config.json" ]; then
|
|||||||
fi
|
fi
|
||||||
|
|
||||||
# Set proper permissions for secrets file (restrictive: owner rw, group r)
|
# 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
|
# Owned by whoever WRITES the file, which is the web interface.
|
||||||
# Otherwise, use ACTUAL_USER and rely on group membership
|
#
|
||||||
|
# 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
|
if [ -f "$PROJECT_ROOT_DIR/config/config_secrets.json" ]; then
|
||||||
# Check if service runs as root (from service file or template)
|
# The web service is the writer; fall back to the display service, then to
|
||||||
SERVICE_USER="root"
|
# the installing user, so an unusual layout still lands somewhere sensible.
|
||||||
if [ -f "/etc/systemd/system/ledmatrix.service" ]; then
|
SECRETS_OWNER=""
|
||||||
SERVICE_USER=$(grep "^User=" /etc/systemd/system/ledmatrix.service | cut -d'=' -f2 || echo "root")
|
for unit in "/etc/systemd/system/ledmatrix-web.service" \
|
||||||
elif [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix.service" ]; then
|
"$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service"; do
|
||||||
SERVICE_USER=$(grep "^User=" "$PROJECT_ROOT_DIR/systemd/ledmatrix.service" | cut -d'=' -f2 || echo "root")
|
if [ -f "$unit" ]; then
|
||||||
|
SECRETS_OWNER=$(grep -m1 "^User=" "$unit" | cut -d'=' -f2)
|
||||||
|
[ -n "$SECRETS_OWNER" ] && break
|
||||||
fi
|
fi
|
||||||
|
done
|
||||||
if [ "$SERVICE_USER" = "root" ]; then
|
if [ -z "$SECRETS_OWNER" ]; then
|
||||||
# Service runs as root - set ownership to root so it can read as owner
|
SECRETS_OWNER="$ACTUAL_USER"
|
||||||
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)"
|
|
||||||
fi
|
fi
|
||||||
|
# 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"
|
chmod 640 "$PROJECT_ROOT_DIR/config/config_secrets.json"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
|||||||
+77
-1
@@ -16,6 +16,7 @@ import json
|
|||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
import shutil
|
import shutil
|
||||||
|
import stat
|
||||||
import socket
|
import socket
|
||||||
import tempfile
|
import tempfile
|
||||||
import zipfile
|
import zipfile
|
||||||
@@ -82,6 +83,10 @@ BUNDLED_FONTS: frozenset[str] = frozenset({
|
|||||||
_CONFIG_REL = Path("config/config.json")
|
_CONFIG_REL = Path("config/config.json")
|
||||||
_SECRETS_REL = Path("config/config_secrets.json")
|
_SECRETS_REL = Path("config/config_secrets.json")
|
||||||
_WIFI_REL = Path("config/wifi_config.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")
|
_FONTS_REL = Path("assets/fonts")
|
||||||
_PLUGIN_UPLOADS_REL = Path("assets/plugins")
|
_PLUGIN_UPLOADS_REL = Path("assets/plugins")
|
||||||
_STATE_REL = Path("data/plugin_state.json")
|
_STATE_REL = Path("data/plugin_state.json")
|
||||||
@@ -303,6 +308,9 @@ def create_backup(
|
|||||||
if (project_root / _WIFI_REL).exists():
|
if (project_root / _WIFI_REL).exists():
|
||||||
zf.write(project_root / _WIFI_REL, _WIFI_REL.as_posix())
|
zf.write(project_root / _WIFI_REL, _WIFI_REL.as_posix())
|
||||||
contents.append("wifi")
|
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-uploaded fonts.
|
||||||
user_fonts = iter_user_fonts(project_root)
|
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_config": (project_root / _CONFIG_REL).exists(),
|
||||||
"has_secrets": (project_root / _SECRETS_REL).exists(),
|
"has_secrets": (project_root / _SECRETS_REL).exists(),
|
||||||
"has_wifi": (project_root / _WIFI_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)],
|
"user_fonts": [p.name for p in iter_user_fonts(project_root)],
|
||||||
"plugin_uploads": len(iter_plugin_uploads(project_root)),
|
"plugin_uploads": len(iter_plugin_uploads(project_root)),
|
||||||
"plugins": list_installed_plugins(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")
|
detected.append("secrets")
|
||||||
if _WIFI_REL.as_posix() in names:
|
if _WIFI_REL.as_posix() in names:
|
||||||
detected.append("wifi")
|
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):
|
if any(n.startswith(_FONTS_REL.as_posix() + "/") for n in names):
|
||||||
detected.append("fonts")
|
detected.append("fonts")
|
||||||
if any(
|
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:
|
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)
|
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(
|
def restore_backup(
|
||||||
@@ -546,6 +610,18 @@ def restore_backup(
|
|||||||
elif (tmp_dir / _WIFI_REL).exists():
|
elif (tmp_dir / _WIFI_REL).exists():
|
||||||
result.skipped.append("wifi")
|
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.
|
# User fonts — skip anything that collides with a bundled font.
|
||||||
tmp_fonts = tmp_dir / _FONTS_REL
|
tmp_fonts = tmp_dir / _FONTS_REL
|
||||||
if options.restore_fonts and tmp_fonts.exists():
|
if options.restore_fonts and tmp_fonts.exists():
|
||||||
|
|||||||
@@ -1143,7 +1143,7 @@ class PluginStoreManager:
|
|||||||
"""
|
"""
|
||||||
registry = self.fetch_registry()
|
registry = self.fetch_registry()
|
||||||
plugins = registry.get('plugins', []) or []
|
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:
|
if not plugin_info:
|
||||||
return None
|
return None
|
||||||
@@ -1183,6 +1183,37 @@ class PluginStoreManager:
|
|||||||
|
|
||||||
return plugin_info
|
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]:
|
def get_registry_info(self, plugin_id: str) -> Optional[Dict]:
|
||||||
"""
|
"""
|
||||||
Get plugin information from the registry cache only (no GitHub API calls).
|
Get plugin information from the registry cache only (no GitHub API calls).
|
||||||
@@ -1198,7 +1229,7 @@ class PluginStoreManager:
|
|||||||
"""
|
"""
|
||||||
registry = self.fetch_registry()
|
registry = self.fetch_registry()
|
||||||
plugins = registry.get('plugins', []) or []
|
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:
|
def install_plugin(self, plugin_id: str, branch: Optional[str] = None) -> bool:
|
||||||
"""Install a plugin, keeping any existing install until the new one is
|
"""Install a plugin, keeping any existing install until the new one is
|
||||||
|
|||||||
@@ -3,6 +3,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import json
|
import json
|
||||||
|
import stat
|
||||||
import zipfile
|
import zipfile
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
@@ -41,6 +42,13 @@ def _make_project(root: Path) -> Path:
|
|||||||
json.dumps({"ap_mode": {"ssid": "LEDMatrix"}}),
|
json.dumps({"ap_mode": {"ssid": "LEDMatrix"}}),
|
||||||
encoding="utf-8",
|
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 = root / "assets" / "fonts"
|
||||||
fonts.mkdir(parents=True)
|
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())
|
restored_secrets = json.loads((empty_project / "config" / "config_secrets.json").read_text())
|
||||||
assert restored_secrets["ledmatrix-weather"]["api_key"] == "SECRET"
|
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.
|
# 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" / "my-custom-font.ttf").read_bytes() == b"\x00\x01USER"
|
||||||
assert (empty_project / "assets" / "fonts" / "5x7.bdf").read_text() == "BUNDLED"
|
assert (empty_project / "assets" / "fonts" / "5x7.bdf").read_text() == "BUNDLED"
|
||||||
@@ -282,3 +294,39 @@ def test_restore_rejects_malicious_zip(empty_project: Path, tmp_path: Path) -> N
|
|||||||
# validate_backup catches it before extraction.
|
# validate_backup catches it before extraction.
|
||||||
assert not result.success
|
assert not result.success
|
||||||
assert any("unsafe" in e.lower() for e in result.errors)
|
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
|
||||||
|
|||||||
@@ -1,96 +0,0 @@
|
|||||||
"""Guard: enum dropdowns in the plugin config form honour x-options.labels.
|
|
||||||
|
|
||||||
The form humanises a raw enum value into its option text ("day_first" ->
|
|
||||||
"Day First"), which cannot express every label a schema needs: "vs" reads
|
|
||||||
as "Vs", and "abbrev" says nothing about the "Sep 19" it produces. Schemas
|
|
||||||
can supply x-options.labels instead, the same convention the checkbox-group
|
|
||||||
widget already uses.
|
|
||||||
|
|
||||||
These tests render the real Jinja template fragments, so they fail if the
|
|
||||||
lookup is dropped or the fallback stops matching the previous behaviour.
|
|
||||||
"""
|
|
||||||
from pathlib import Path
|
|
||||||
|
|
||||||
import pytest
|
|
||||||
from jinja2 import DictLoader, Environment
|
|
||||||
|
|
||||||
PROJECT_ROOT = Path(__file__).resolve().parent.parent
|
|
||||||
CONFIG_FORM = (PROJECT_ROOT / 'web_interface' / 'templates' / 'v3' / 'partials'
|
|
||||||
/ 'plugin_config.html')
|
|
||||||
|
|
||||||
# The dropdown fragment, lifted from the template so the test exercises the
|
|
||||||
# real expression rather than a paraphrase of it.
|
|
||||||
SELECT_FRAGMENT = """
|
|
||||||
{%- set enum_labels = (prop.get('x-options') or prop.get('x_options') or {}).get('labels') or {} -%}
|
|
||||||
{%- for option in prop.enum -%}
|
|
||||||
<option value="{{ option }}">{{ enum_labels.get(option, option|replace('_', ' ')|title) }}</option>
|
|
||||||
{%- endfor -%}
|
|
||||||
"""
|
|
||||||
|
|
||||||
|
|
||||||
def _render(prop: dict) -> str:
|
|
||||||
env = Environment(loader=DictLoader({'f': SELECT_FRAGMENT}), autoescape=True)
|
|
||||||
return env.get_template('f').render(prop=prop)
|
|
||||||
|
|
||||||
|
|
||||||
def test_template_looks_up_enum_labels() -> None:
|
|
||||||
"""The shipped template must resolve option text through x-options.labels."""
|
|
||||||
source = CONFIG_FORM.read_text(encoding='utf-8')
|
|
||||||
assert "enum_labels.get(option," in source, (
|
|
||||||
'plugin_config.html no longer resolves enum option text through '
|
|
||||||
'x-options.labels; schemas that supply labels would silently show '
|
|
||||||
'raw values again'
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def test_labels_are_used_when_supplied() -> None:
|
|
||||||
prop = {
|
|
||||||
'enum': ['vs', 'date_time'],
|
|
||||||
'x-options': {'labels': {'vs': 'VS', 'date_time': 'Date and time'}},
|
|
||||||
}
|
|
||||||
html = _render(prop)
|
|
||||||
assert '>VS<' in html
|
|
||||||
assert '>Date and time<' in html
|
|
||||||
|
|
||||||
|
|
||||||
def test_unlabelled_values_keep_the_humanised_fallback() -> None:
|
|
||||||
"""Schemas without labels must render exactly as they did before."""
|
|
||||||
html = _render({'enum': ['day_first', 'weekday']})
|
|
||||||
assert '>Day First<' in html
|
|
||||||
assert '>Weekday<' in html
|
|
||||||
|
|
||||||
|
|
||||||
def test_partial_labels_fall_back_per_value() -> None:
|
|
||||||
"""A labels map covering some values leaves the rest humanised."""
|
|
||||||
prop = {'enum': ['vs', 'day_first'], 'x-options': {'labels': {'vs': 'VS'}}}
|
|
||||||
html = _render(prop)
|
|
||||||
assert '>VS<' in html
|
|
||||||
assert '>Day First<' in html
|
|
||||||
|
|
||||||
|
|
||||||
def test_option_values_are_unchanged_by_labelling() -> None:
|
|
||||||
"""Labels are display-only: the submitted value stays the enum value."""
|
|
||||||
prop = {'enum': ['abbrev'], 'x-options': {'labels': {'abbrev': 'Sep 19'}}}
|
|
||||||
html = _render(prop)
|
|
||||||
assert 'value="abbrev"' in html
|
|
||||||
assert '>Sep 19<' in html
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize('key', ['x-options', 'x_options'])
|
|
||||||
def test_both_option_key_spellings_work(key: str) -> None:
|
|
||||||
"""The template accepts either spelling, as its other widgets do."""
|
|
||||||
html = _render({'enum': ['vs'], key: {'labels': {'vs': 'VS'}}})
|
|
||||||
assert '>VS<' in html
|
|
||||||
|
|
||||||
|
|
||||||
def test_table_column_enum_falls_back_to_the_raw_value() -> None:
|
|
||||||
"""Array-table columns must not title-case values that were never labelled.
|
|
||||||
|
|
||||||
Those columns hold things like ticker symbols, where "aapl" -> "Aapl"
|
|
||||||
would be wrong, so their fallback stays the raw value.
|
|
||||||
"""
|
|
||||||
source = CONFIG_FORM.read_text(encoding='utf-8')
|
|
||||||
assert 'col_labels.get(opt, opt)' in source, (
|
|
||||||
'array-table column options must fall back to the raw value, not the '
|
|
||||||
'humanised one'
|
|
||||||
)
|
|
||||||
@@ -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"
|
||||||
@@ -7848,7 +7848,31 @@ def clear_old_errors():
|
|||||||
# Backup / Restore
|
# 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"
|
||||||
|
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:
|
def _safe_backup_path(filename: str) -> Path:
|
||||||
@@ -8002,7 +8026,22 @@ def backup_restore():
|
|||||||
|
|
||||||
data = result.to_dict()
|
data = result.to_dict()
|
||||||
if not result.success:
|
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})
|
return jsonify({'status': 'success', 'data': data})
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error("backup_restore failed: %s", e, exc_info=True)
|
logger.error("backup_restore failed: %s", e, exc_info=True)
|
||||||
|
|||||||
@@ -121,21 +121,14 @@
|
|||||||
</label>
|
</label>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
|
||||||
{# Enum dropdown. Option text comes from x-options.labels when the
|
{# Enum dropdown #}
|
||||||
schema supplies it -- the same convention the checkbox-group
|
|
||||||
widget already uses -- because humanising the raw value cannot
|
|
||||||
express every label: "vs" reads as "Vs", and "abbrev" says
|
|
||||||
nothing about the "Sep 19" it produces. Values without a label
|
|
||||||
fall back to the humanised form, so existing schemas render
|
|
||||||
exactly as before. #}
|
|
||||||
{% elif prop.enum %}
|
{% elif prop.enum %}
|
||||||
{% set enum_labels = (prop.get('x-options') or prop.get('x_options') or {}).get('labels') or {} %}
|
|
||||||
<select id="{{ field_id }}"
|
<select id="{{ field_id }}"
|
||||||
name="{{ full_key }}"
|
name="{{ full_key }}"
|
||||||
class="form-select w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 bg-white text-black">
|
class="form-select w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 bg-white text-black">
|
||||||
{% for option in prop.enum %}
|
{% for option in prop.enum %}
|
||||||
<option value="{{ option }}" {% if value == option %}selected{% endif %}>
|
<option value="{{ option }}" {% if value == option %}selected{% endif %}>
|
||||||
{{ enum_labels.get(option, option|replace('_', ' ')|title) }}
|
{{ option|replace('_', ' ')|title }}
|
||||||
</option>
|
</option>
|
||||||
{% endfor %}
|
{% endfor %}
|
||||||
</select>
|
</select>
|
||||||
@@ -576,14 +569,10 @@
|
|||||||
class="block w-20 px-2 py-1 border border-gray-300 rounded text-sm text-center"
|
class="block w-20 px-2 py-1 border border-gray-300 rounded text-sm text-center"
|
||||||
{% if col_def.get('description') %}title="{{ col_def.get('description') }}"{% endif %}>
|
{% if col_def.get('description') %}title="{{ col_def.get('description') }}"{% endif %}>
|
||||||
{% elif col_enum %}
|
{% elif col_enum %}
|
||||||
{# Labels are opt-in here and the fallback stays the raw
|
|
||||||
value: table columns hold things like ticker symbols,
|
|
||||||
which must not be title-cased behind the user's back. #}
|
|
||||||
{% set col_labels = (col_def.get('x-options') or col_def.get('x_options') or {}).get('labels') or {} %}
|
|
||||||
<select name="{{ full_key }}.{{ item_index }}.{{ col_name }}"
|
<select name="{{ full_key }}.{{ item_index }}.{{ col_name }}"
|
||||||
class="block w-full px-2 py-1 border border-gray-300 rounded text-sm bg-white">
|
class="block w-full px-2 py-1 border border-gray-300 rounded text-sm bg-white">
|
||||||
{% for opt in col_enum %}{% if opt is not none %}
|
{% for opt in col_enum %}{% if opt is not none %}
|
||||||
<option value="{{ opt }}" {% if col_value == opt or (col_value is none and col_def.get('default') == opt) %}selected{% endif %}>{{ col_labels.get(opt, opt) }}</option>
|
<option value="{{ opt }}" {% if col_value == opt or (col_value is none and col_def.get('default') == opt) %}selected{% endif %}>{{ opt }}</option>
|
||||||
{% endif %}{% endfor %}
|
{% endif %}{% endfor %}
|
||||||
</select>
|
</select>
|
||||||
{% elif col_xwidget == 'date-picker' %}
|
{% elif col_xwidget == 'date-picker' %}
|
||||||
|
|||||||
Reference in New Issue
Block a user