fix(web): mask the Config Editor's secrets; keep disabled plugins' rotation slot and Vegas exclusion; restore only missing plugins - #743
Merged
Conversation
The Config Editor tab (/partials/raw-json) filled its config_secrets.json editor with the file as it is on disk. GET /api/v3/config/secrets masks every value because the interface is reachable without a login by default, but this page handed the same credentials (GitHub token, Home Assistant token, plugin API keys) to anyone who loaded it. The masked-save path in save_raw_secrets_config was written for a masked editor and never got one. _load_raw_json_partial now masks the section with mask_all_secret_values after strip_auth_section, exactly as the GET does. Saving it back is safe: save_raw_secrets_config drops the masks (strip_masked_values) and merges the rest onto the stored file (deep_merge), so an untouched secret stays as it is and a replaced mask is the only value that changes. The config.json editor is left as it is. Its save (save_raw_main_config) writes the posted object verbatim, with no mask stripping or merge, so a masked main editor would write the bullets over any credential it holds. Masking it needs a merge-on-save of its own first. Tests: TestConfigEditorRoundTrip renders the partial over a real ConfigManager, checks no real value is in the editor, and posts the editor back unchanged (the file is identical) and with one mask replaced (only that value changes). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… exclusions PluginOrderList draws one row per enabled plugin and, once drawn, rewrites its hidden inputs (plugin_rotation_order, vegas_plugin_order, vegas_excluded_plugins) from those rows. A disabled plugin has no row, so merely opening the Display or Rotation & Durations tab took it out of the inputs, and the next save of that form stored the lists without it. Exclude Clock from Vegas, disable it, change the brightness, re-enable it: Clock was scrolling in Vegas again and had moved to the end of the rotation. syncInputs now keeps the saved ids that have no row. In the order, each one keeps its saved slot and the rows fill the other slots in their current order, with rows not in the saved order last, as before. In the exclusions they follow the unchecked rows. Only string ids are carried over, once each: /config/main refuses a list holding anything else, which would block every later save of the tab. Tests: test/js/unit/test_plugin_order_list.js runs the shipped widget in a vm with a fake DOM (draw, reorder, include/exclude, the rotation list, junk ids) and is in run_all.js and the README. The durations DOM suite now reads only its own rows' ids from the input, since a rig's saved order can hold others. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
POST /backup/restore with reinstall_plugins (the "Reinstall missing plugins" box) passed every plugin in the backup's plugins.json to install_plugin(). That replaces an installed copy with a fresh download, so a restore onto the same device re-downloaded every plugin inside the request. A plugin installed from its own URL is not in the registry, so its install returned False, plugins_failed set success to False, and the restore answered 500 "Restore incomplete ... plugins not reinstalled: <id>" (shown as "Restore failed") with the plugin still installed and the config restored. Each plugin is now looked up first with the store's _existing_install, the same lookup install_plugin makes to decide a copy exists: the id, or an id the registry proves is the same plugin (aliases, the plugin_path name), and never a bare ledmatrix-<id> folder (#686). One that is installed is recorded in result.skipped as "plugin:<id> (installed)", which the page lists under Skipped; a missing one is installed as before. The list_installed_plugins docstring said every listed plugin is reinstalled and now says otherwise. Tests: TestInstalledPluginsAreNotReinstalled, with a mocked store (installed skipped, missing installed; an installed plugin the store can't install is not a failure) and with a real PluginStoreManager (a registry alias and a third-party install are skipped, a missing plugin installed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
save_main_config read a JSON body with request.get_json(), which raises
Werkzeug's BadRequest for a body that does not parse (or an empty one sent as
application/json). That happened inside the handler's try, so the
catch-all answered 500 CONFIG_SAVE_FAILED with "Check file permissions on
config directory" among its suggested fixes and logged a traceback at
ERROR, for what was the caller's mistake.
It now reads with get_json(silent=True), as save_raw_main_config does, and
answers a sent-but-unparseable body with the same 400
{"status": "error", "message": "Invalid JSON in request body"}. An empty
JSON body falls through to the existing 400 "No data provided". The change
is limited to the lines that read the body.
Tests: TestMalformedBody in test_api_v3_partial_main_save.py (the 400 and its
shape, identical to /config/raw/main's, and nothing saved; the empty body).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GET /api/v3/fonts/catalog caches its answer as fonts_catalog for five minutes. Font upload and delete clear that entry (fonts.py), but POST /backup/restore copies user fonts into assets/fonts without touching it, so restored fonts were missing from the Fonts tab and every font picker until the cache expired. backup_restore now clears fonts_catalog when the result lists restored fonts (restore_backup records them as "fonts (<count>)"). A restore that restored no fonts leaves the cache alone. Tests: TestFontsCatalogCache in test_api_v3_backup_restore.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…clusions 2b34f25 made the plugin order list keep every saved id that has no row, so a disabled plugin keeps its rotation slot and Vegas exclusion. That also kept the ids of plugins that have since been uninstalled: they stayed in plugin_rotation_order and vegas_excluded_plugins for good, where before the next save of the tab dropped them. The widget already fetches /api/v3/plugins/installed, every installed plugin with its enabled flag, and draws only the enabled ones. It now keeps that response's full id set and carries over only saved ids that are installed but have no row (disabled). An id outside the set is dropped, as before. With no list, nothing is dropped: a failed request draws no rows and leaves the inputs as saved, and the carry-over keeps everything if the set was never filled. Tests: test/js/unit/test_plugin_order_list.js adds a disabled plugin kept while an uninstalled one is dropped (order and exclusions; fails on 2b34f25), and a failed plugin list leaving both inputs as saved. The CHANGELOG bullet and the README row say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ites Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 29 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (13)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 19 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five web bugs, one commit each.
config_secrets.jsonin plain text (pages_v3._load_raw_json_partial)./v3/partials/raw-json. MeanwhileGET /api/v3/config/secretsmasks them on purpose, and the raw-secrets save path was written to accept masked values.mask_all_secret_values(strip_auth_section(...)).config.jsoneditor is unchanged: its save writes verbatim, so masks would overwrite real values.vegas_excluded_pluginsandplugin_rotation_order(plugin-order-list.js)._existing_installlookup (registry aliases count, a bareledmatrix-<id>folder does not). They are reported as skipped. Missing ones are still installed.POST /api/v3/config/mainanswered malformed JSON with a 500, plus "check file permissions" advice. It now returns a 400 matching/config/raw/main.fonts_catalogcache, as upload and delete already do.Tests
TestConfigEditorRoundTrip(3),TestInstalledPluginsAreNotReinstalled(3),TestMalformedBody(2),TestFontsCatalogCache(2). All fail on main except guards.unit/test_plugin_order_list.js(14 checks), run bynode test/js/run_all.js.••••••••for both seeded secrets, and the real values appear nowhere in the page;config_secrets.jsonholding the real values.On ledpi: the Config Editor page held none of the device's 3 real secrets.
Part of a bug sweep
This is one of 10 independent fix PRs from one sweep, all based on
mainef69201.main: the same 62 failures and 6 errors on both. These are the known Windows path and file-locking tests. 191 more tests pass.test_backup_manager.py::test_create_backup_contents(os.replace→WinError 5on a temp zip), was a Windows file-lock flake. It passes on rerun, and nothing here touchescreate_backup.main, with a clean start and no errors or render stalls in the journal. ledpi is back on plainmain.🤖 Generated with Claude Code