4 Commits

Include renames

Author	SHA1	Message	Date
ChuckandClaude Opus 5.5	a8b3e86775	refactor(web): delete dead routes, JS files and duplicate definitions (#609) ... * refactor(web): drop validators nothing calls escape_html, validate_image_url, validate_font_awesome_class, validate_mime_type, validate_numeric_range, validate_string_length and sanitize_plugin_config had no callers outside their own tests. Only validate_file_upload (fonts upload) is imported by the web interface. dedup_unique_arrays is kept: its one caller in save_plugin_config was removed by the unrelated sync PR (#330), which looks accidental. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(api): remove the music-auth and of-the-day JSON routes POST /plugins/authenticate/spotify and /plugins/authenticate/ytm had no caller but their tests: the music plugin authenticates through its web_ui_actions (authenticate_spotify.py / authenticate_ytm.py) via /plugins/action. POST /plugins/of-the-day/json/upload and /json/delete looked the plugin up by the id ledmatrix-of-the-day (its manifest id is of-the-day), were reachable only from a file_type "json" upload field that no schema declares, and put the plugin directory on sys.path per request to import scripts.update_config. of-the-day manages its files through plugin-file-manager and its own web_ui_actions. The of-the-day branch of GET /plugins/config stays: it matches the real manifest id and still merges the on-disk category files into the form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(api): read managers only from the blueprints api_v3/__init__.py and pages_v3.py declared module globals (plugin_store_manager, saved_repositories_manager, schema_manager, operation_queue, plugin_state_manager, operation_history, sync_manager, config_manager, plugin_manager) that nothing assigns: app.py sets the managers as attributes on the Blueprint objects, and every route reads them there. The one reader, backup restore's fallback to the module plugin_store_manager, could only ever fall back to None. _ensure_cache_manager() built a second CacheManager in the web process instead of using the one app.py puts on api_v3. The display routes now read api_v3.cache_manager, creating it on the blueprint only when nothing set it (the same None handling as the /cache routes). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(web): drop run.sh and the unused log_config_change web_interface/run.sh was referenced only by web_interface/README.md; the service starts the UI through scripts/utils/start_web_conditionally.py and the README already documents `python3 web_interface/start.py`. log_config_change() in web_interface/logging_config.py was never called. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(web): delete unreferenced store_manager.js, diff_viewer.js, htmx-sse.js - js/plugins/store_manager.js (window.PluginStoreManager) and js/config/diff_viewer.js (window.ConfigDiffViewer) were loaded on every page but nothing reads either global. - htmx-sse.js (plus its CDN fallback) was loaded after HTMX, but no template or plugin page uses sse-connect / hx-ext="sse": the live streams run through LEDStreams in app-shell.js. js/plugins/state_manager.js stays: install_manager.js's updateAll() reads and refreshes window.PluginStateManager. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(web): remove app.js helpers nothing calls - hexToRgb, rgbToHex, validateForm, uploadFont and switchTab (whose 'switch-tab' event had no listener) have no caller in the templates, static JS or the plugin monorepo. - installPlugin: plugins_manager.js (loaded last) assigns window.installPlugin, and its own store cards are the only callers. - The showNotification fallback could never install: app-shell.js is deferred ahead of app.js and defines the same fallback at top level. - performanceMonitor only logged with ?debug=perf and read an unset this.measures; the marks it took on every load had no reader. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(web): drop app-shell.js refreshPlugin A top-level function in app-shell.js, so a window global, but nothing calls it (no inline handler, no window lookup, no string-built name). The other plugin actions in that block stay. updatePlugin is the live window.updatePlugin: plugins_manager.js only installs its own copy when none exists. uninstallPlugin/pollUninstallOperation, updateAllPlugins, executePluginAction and toggleNestedSection are replaced by later deferred scripts, but a click that lands while those scripts are still downloading reaches the app-shell copies, so removing them is not a pure no-op. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(web): remove definitions plugins_manager.js always overrides All of these are replaced before anything can call them, checked against the load order in base.html and the live window.* values: - openOnDemandModal/requestOnDemandStop stubs: the IIFE later in the same script assigns the real functions synchronously. - updatePlugin and uninstallPlugin stubs (`window.X || stub`): app-shell.js already defined both, so the fallback never installed. Same for the later updatePlugin override, gated on the live function containing '[UPDATE]', which app-shell.js's never does. - The first addArrayObjectItem/removeArrayObjectItem: reassigned by the top-level copies after the IIFE. - The first `function formatDate` in the IIFE: a later declaration of the same name in the same scope wins. - deleteUploadedImage, getCurrentImages, showUploadProgress, formatFileSize and getScheduleSummary: character-for-character copies of js/widgets/file-upload.js, which stays the owner. - `typeof X === 'undefined'` fallbacks and `typeof X !== 'undefined'` re-exports after the IIFE: always false, or a self-assignment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(web): render the shell directly and delete index.html index.html extended base.html with {% block content %}, but base.html defines no blocks, so none of index.html ever rendered: rendering both with jinja2 gives byte-identical output. index() still loaded the config, read config.json and config_secrets.json raw and json.dumps'd them on every page load for variables base.html never reads, and flashed errors that base.html never shows. It now renders base.html with no context. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): stop htmx-config.js replacing console.error and console.warn It swapped both globals for filters that dropped any error mentioning insertBefore / "Cannot read properties of null" when "htmx" appeared in the message or stack, and a list of Permissions-Policy warnings. That hid real errors from every script on the page, and made every logged error and warning report htmx-config.js as its source. The beforeSwap target validation above it, which prevents the insertBefore errors in the first place, stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(web): quiet the widget load announcements and debug logs About 30 lines hit the console on every page load: one "... widget registered" per widget file, one "[WidgetRegistry] Registered widget: X" per registration, plus the registry, base widget and plugin loader announcing themselves. The load-time announcements are removed; the per-call ones (registry register, plugin widget loads, "Render called") now go through the page's debugLog switch (localStorage.pluginDebug), guarded because the widgets also load in node tests without it. fonts.html and wifi.html debug logging goes through debugLog as well. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(api): drop the removed music-auth and of-the-day JSON routes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-09-23 12:36:53 -04:00
ChuckandClaude Opus 5	f6367d63ae	security: triage the CodeQL backlog — 129 alerts, three of them live (#561) ... * fix(web): escape quotes in every HTML escaper, not just & < > The escapers are all `div.textContent = x; return div.innerHTML`. That round-trip escapes &, < and > -- the only characters the HTML serializer must escape in a text node -- and leaves quotes alone. Every widget then interpolates the result into a quoted attribute value: value="${escapeHtml(v)}" title="${escapeHtml(v)}" so a value of `x" onmouseover="alert(1)` closes the attribute and adds an event handler of its own. CodeQL reported this 83 times (js/incomplete-html-attribute-sanitization) across the widget files. It is one bug, not 83: the widgets each carry a standalone fallback that did escape quotes, but they all prefer BaseWidget.escapeHtml when window.BaseWidget exists -- which it always does in the shipped page -- so the correct fallbacks were dead code and the incomplete shared one ran. Fixed at each source instead of at the call sites. app-shell.js already documented this exact gap in a comment and worked around it by building DOM nodes by hand; that workaround stays (setting a property cannot be got wrong), the comment is now accurate. cache.html's delete button interpolated the cache key into `onclick="deleteCacheFile('...')"`. Escaping cannot help there -- the browser HTML-decodes the attribute before parsing it as JS, so `&#39;` becomes a real `'` again -- so the key moves to a data-cache-key attribute that the handler reads back. url-input.js additionally wrote a value straight into an <a href> after validating it against a schema-supplied protocol list, and that list accepted any RFC 3986 scheme -- "javascript" included. Scriptable schemes (javascript, data, vbscript, blob, filesystem) are now refused both when the list is normalised and when a URL is checked against it, and the render path routes its href through the same check instead of emitting whatever was stored (js/xss-through-dom). test/js/unit/test_html_escaping.js reads each escaper out of the shipped file and runs it, so losing the quote handling again fails a test rather than a scan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(security): stop request-supplied names from reaching paths outside their base Three of the py/path-injection alerts were live, not lint: * GET /api/v3/plugins/<plugin_id>/static/<path:file_path> read any file whose resolved path *string-prefixed* the plugin directory. Flask's default converter forbids a slash but not dots, and get_plugin_directory('..') returned the parent of the plugins directory because it exists -- so every file under the project root then prefixed that directory, config/config_secrets.json included. The prefix check was also wrong on its own terms: with plugin dir "plugin-repos/foo", "../foo-evil/x" resolves to "plugin-repos/foo-evil/x", whose string does start with "plugin-repos/foo". * POST /api/v3/plugins/of-the-day/json/delete interpolated the request body's file_id into f"{file_id}.json" and unlinked it, unvalidated. A file_id of "../../../../etc/something" deleted that file. This is the one finding in the batch that destroyed data rather than exposing it. * POST /api/v3/cache/delete passed the body's key through CacheManager.clear_cache to DiskCache, which joined it as a filename and called os.remove. Same shape, same result. The guard goes in DiskCache.get_cache_path, the single choke point get/set/clear share, so every caller is covered rather than just this route. Real keys are the stems of files already flat in the cache directory -- that is how list_cache_files derives them -- so nothing legitimate is turned away. The rest of the cluster (web_interface/app.py's asset route, the plugin update handler, _get_plugin_version, the plugin-schema read in config.py) was guarded in ways that held, but each had grown its own version of the check. They now go through one helper, src/common/path_safety.py, which returns the *sanitised value* rather than a verdict -- so a caller cannot validate one string and open another, which is how the two real bugs above were shaped. Also: WiFiManager.connect_to_network took the SSID and password straight from POST /api/v3/wifi/connect into nmcli's argv. There is no shell there, so CodeQL's py/command-line-injection alert overstates the risk -- but nmcli reads a leading "-" as an option, so an SSID of "--ask" asks nmcli to run differently rather than to join a network. Both values are now checked for shape (802.11's 32-octet SSID limit, WPA's 8-63 char passphrase or 64-char hex key, no control characters, no leading dash) before any subprocess runs. test/test_path_traversal_guards.py asserts on the filesystem, not just the status code: a handler that returns 403 and deletes the file anyway would pass the weaker check. Twelve of its cases fail against the unpatched code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(web): refuse a plugin id that is not a plain name, don't truncate it pages_v3 and scripts/dev_server.py ran request ids through os.path.basename and carried on with what came out, so "../weather" rendered the config form for "weather". Nothing escaped the plugins directory -- the relative_to guards held -- but the handler answered a request nobody made, and validating one string while the filesystem sees another is the shape both live traversals earlier in this branch had. Same treatment as the rest: safe_path_component rejects rather than truncates, resolve_under returns the path it checked, and the call sites use what those return. The three handlers that had hand-rolled resolve-and-relative_to blocks lose about twenty lines to the shared one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(web): say what the plugin web_ui iframe actually is The docstring claimed the fragment runs "in a sandboxed iframe". The iframe in plugin_config.html carries no sandbox attribute, so the fragment runs with the interface's own origin. That is fine -- the file belongs to an installed plugin, and an installed plugin already runs Python on the device, so the trust boundary is install rather than this route -- but a comment promising containment that is not there is worse than no comment. This is the context for the py/reflective-xss alert on this handler. Also drops the now-unused os/os.path imports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(web): inline url-input's scheme guard at the previewLink.href sink CodeQL flagged this line as a new high-severity js/xss-through-dom alert on this PR even though it is already covered by SCRIPTABLE_SCHEMES: the guard reached the sink through safeHref -> isValidUrl, two function calls away, which its DOM-based-XSS sanitizer recognition does not trace. Behavior is unchanged -- same scheme check, same SCRIPTABLE_SCHEMES list, same allowedProtocols gate -- just inlined directly above the previewLink.href assignment it guards, so the barrier is visible in the same scope as the sink. Added a regression test that runs the shipped onInput handler (not just the extracted helpers) against a mocked DOM, so a future change that reintroduces an unguarded previewLink.href assignment fails here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(security): address CodeRabbit findings on the CodeQL triage PR - src/wifi_manager.py: reject non-ASCII WPA-PSK passphrases before any credential-saving or connect flow runs. NetworkManager only accepts printable ASCII passphrases (or a 64-char hex key); a non-ASCII value was previously saved/attempted before nmcli itself rejected it. - web_interface/blueprints/api_v3/config.py: fail closed when the plugin config schema path can't be resolved under the plugins directory (e.g. a symlinked plugin dir). Previously this fell through with secret_fields left empty, so submitted credentials for that plugin were saved as ordinary, unencrypted configuration. - web_interface/static/v3/js/widgets/plugin-file-manager.js: stop splicing the JSON day/column key into an inline oninput="..." handler string. escHtml() escapes quotes for a normal HTML attribute, but the browser HTML-decodes the attribute before running it as script, which undoes that escaping and lets a crafted column name (e.g. from an uploaded JSON file) break out of the JS string and execute. Cell edits now travel through data-day/data-col attributes read by one delegated 'input' listener instead. While in this file: fixed 6 pre-existing missing-')' typos on multi-line safeSetHTML(...) calls (already flagged by Biome in this PR's own CodeRabbit run as syntax errors blocking its lint pass). These predate this PR (present on main too) but made the whole file fail to parse in any JS engine, which is a bigger problem than the XSS finding itself and directly touches the same lines. Added/extended regression tests for each fix; full suites pass (pytest: 4580 passed, 62 skipped; JS: 84 assertions). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-11 15:32:03 -04:00
3 people	713539e491	fix(web-ui): fix quick actions not firing, add toast feedback, suppress install handler warning (#346) ... * fix(web-ui): fix quick actions not firing, add toast feedback, suppress install handler warning - base.html: add htmx:afterSettle listener to set data-loaded on tab containers after HTMX swaps their content, preventing the overview partial from being re-fetched (and handlers lost) on every tab switch - base.html: call htmx.process() in loadOverviewDirect/loadPluginsDirect fallbacks so buttons get HTMX handlers even if HTMX finished its initial body scan before the fallback fetch completed - overview.html + index.html (11 buttons): replace event.detail.xhr.responseJSON (undefined in HTMX 1.9.x) with JSON.parse(event.detail.xhr.responseText) so quick action toast notifications actually fire - plugins_manager.js: add guarded htmx:afterSettle listener that only calls attachInstallButtonHandler when #install-plugin-from-url is in the DOM, eliminating the spurious console warning on non-plugin tab loads Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web-ui): ensure quick-action toasts always fire even on xhr/parse failure Replace silent catch(e){} in all 11 hx-on:htmx:after-request handlers with a pattern that sets default message/status before the try block and calls showNotification(m,s) unconditionally after it, so a fallback toast is shown whenever xhr is absent or responseText is not valid JSON. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web-ui): show error toast on non-JSON 4xx/5xx quick-action responses In the catch block of all 11 hx-on:htmx:after-request handlers, check xhr.status >= 400 and downgrade s to 'error' so a failed action that returns an HTML error page (or other non-JSON body) surfaces as an error toast instead of the optimistic 'success'/'info' default. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web-ui): guard setTimeout fallback for attachInstallButtonHandler The 500ms fallback setTimeout was calling attachInstallButtonHandler() unconditionally even when the plugins partial wasn't in the DOM, causing a spurious console.warn on every page load. Add the same element-existence check already present on the htmx:afterSettle listener. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix backup API 404s, hardware status 500, and HTMX loading race - Add all backup API routes to api_v3.py: preview, list, export, validate, restore (with plugin reinstall), download, delete - Fix PermissionError on /hardware/status: return graceful 200 instead of 500 when the status file is owned by a different user; also fix root cause by writing the file world-readable (0o644) in display_manager - Fix HTMX race: dispatch htmx:ready window event from HTMX onload callback; loadTabContent now waits for that event instead of immediately falling back to direct fetch (eliminating the "HTMX not available" console warning on initial load) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Cancel HTMX fallback timers when htmx:ready fires The 5-second setTimeout fallbacks for plugins and overview were firing before the htmx:ready event arrived, logging spurious warnings. Each timer now self-cancels via htmx:ready so the fallback only triggers when HTMX genuinely fails to load. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review feedback: error leaks, ok:false, htmx:ready coverage - Backup endpoints: replace raw str(e) in user-facing responses with a generic message; full exception still logged via exc_info=True - hardware/status: change ok:null to ok:false for PermissionError and json.JSONDecodeError so the UI's hw.ok===false check triggers correctly - base.html: dispatch htmx:ready from the fallback load path so any deferred listeners fire on CDN-fallback loads too - loadTabContent: also listen for htmx-load-failed so overview/wifi/plugins fall back to direct fetch when HTMX is completely unavailable Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Treat system-managed pip packages as satisfied for dependency marker When a plugin's requirements.txt includes a package installed via the system package manager (dnf/apt), pip fails with 'uninstall-no-record-file' because it can't replace the system-tracked copy. The package is present and functional, but the missing marker caused the install to be retried on every service restart. Detect this specific error pattern: if the only pip failure is uninstall-no-record-file, write the .dependencies_installed marker and log a warning instead of returning False, suppressing the repeated warning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix uninstall-no-record-file detection condition The previous check used a string replacement that left 'error:' in the remaining text, causing the condition to always evaluate false. Simplify to a direct substring check: if 'uninstall-no-record-file' appears in pip stderr the affected package is installed at the system level and we write the marker, suppressing the repeated warning on every restart. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Resolve CodeQL security findings in backup API Path traversal (CWE-22): - backup_download: switch from send_file(user-tainted-path) to send_from_directory(_BACKUP_EXPORT_DIR, filename); Flask uses werkzeug safe_join internally which CodeQL recognises as a sanitizer - backup_delete: enumerate the export directory and match by name so entry.unlink() operates on a filesystem-derived Path rather than one constructed from user input; _safe_backup_path still guards first Information exposure through exceptions (CWE-209): - backup_validate: err_msg from validate_backup() can embed exception strings containing temp-file paths; log the detail, return a generic 'Invalid or corrupted backup file' to the client - Other backup endpoints: already fixed (str(e) -> generic message); CodeQL alerts will clear on next scan plugin_loader.py:185 (path traversal): false positive — requirements_file is constructed from plugin_dir returned by find_plugin_directory() (a filesystem scan), not from raw HTTP request input; no change needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix pre-existing information exposure in version and action endpoints - get_system_version (alert #218): replaced str(e) with generic message; exception still logged via logger.error(exc_info=True) - execute_system_action (alert #216): removed str(e) and full traceback.format_exc() from the HTTP response — the full stack trace was being sent directly to clients; replaced with generic message and proper logger.error call Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix remaining GitHub CodeQL security alerts - py/stack-trace-exposure: Remove str(e) and traceback.format_exc() from all HTTP responses across api_v3.py, pages_v3.py, and app.py; replace with generic messages and logger.error(exc_info=True) - py/reflective-xss: Escape partial_name via markupsafe.escape in the load_partial 404 response - py/path-injection: Add regex validation of plugin_id before filesystem use in _load_plugin_config_partial - py/incomplete-url-substring-sanitization: Replace 'github.com' in substring checks with urlparse hostname comparison in store_manager.py - py/clear-text-logging-sensitive-data: Remove football-scoreboard debug prints and sensitive request-body prints from update endpoint - js/bad-tag-filter: Replace script-only regex in BaseWidget.sanitizeValue with DOM-based textContent stripping that removes all HTML - js/incomplete-sanitization: Fix escapeAttr to properly encode &, ", ', <, > using HTML entities instead of backslash escaping - js/prototype-pollution-utility: Add __proto__/constructor/prototype key guards to deepMerge function in plugins_manager.js - app.py error handlers: Always return generic messages; remove debug-mode branches that could expose tracebacks in production Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix three remaining CodeQL path-injection and info-exposure alerts - plugin_loader.py: resolve plugin_dir with strict=True and validate marker_path with relative_to() before any filesystem writes, giving CodeQL the positive sanitization pattern it requires (py/path-injection) - api_v3.py _safe_backup_path: replace substring negative checks with a strict positive regex (^[a-zA-Z0-9][a-zA-Z0-9._-]{0,200}\.zip$) that CodeQL recognises as sanitising the user-supplied filename (py/path-injection) - api_v3.py backup_validate: whitelist known-safe manifest fields before returning JSON, preventing any exception strings captured inside validate_backup() from reaching the HTTP response (py/stack-trace-exposure) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Resolve 29 open CodeQL security alerts across 5 files py/flask-debug (#214): - debug_web_manual.py: read debug mode from LEDMATRIX_FLASK_DEBUG env var instead of hardcoded True py/stack-trace-exposure (#216, #218): - api_v3.py execute_system_action: remove subprocess stdout/stderr from HTTP responses; log via logger instead - api_v3.py get_git_version: validate output matches safe ref format (^[a-zA-Z0-9._-]+$) before including in response - api_v3.py: remove all remaining traceback.format_exc() dead variables and print() debug calls (replaced with logger.debug/warning) py/reflective-xss (#207, #208, #209, #210, #211, #212): - api_v3.py: remove plugin_id from all error/success response messages (uninstall, install, update, health, not-found responses) - pages_v3.py load_partial: return static "Partial not found" message instead of echoing partial_name - pages_v3.py _load_starlark_config_partial: add app_id regex validation, use static error messages instead of f-strings with app_id py/path-injection (#187–#206): - pages_v3.py _load_plugin_config_partial: resolve plugins_base and validate _plugin_dir with relative_to() before all file operations; same for assets metadata directory - pages_v3.py _load_starlark_config_partial: resolve starlark_base and validate schema_file/config_file paths with relative_to() - plugin_loader.py _find_plugin_directory: resolve plugins_dir and validate strategy-2 candidates with relative_to() - plugin_loader.py install_dependencies: resolve plugin_dir first, then construct requirements_file and marker_path from resolved base - plugin_loader.py load_module: resolve plugin_dir with strict=True and validate entry_file with relative_to() before exec_module Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix 15 remaining CodeQL path-injection and stack-trace-exposure alerts Switch from resolve()+relative_to() to os.path.basename() reassignment, which CodeQL recognizes as a path sanitizer that breaks the taint chain. Also remove exception objects from backup_manager validate_backup return strings to eliminate the stack-trace-exposure taint source. Fixes alerts #227, #233, #234, #235, #237, #238, #239, #240, #241, #242, #243, #244, #245, #246, #247. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix broken logger format string and leaked exception in config save error - pages_v3.py: plain string was used instead of %-style substitution, so every manifest-read failure logged the literal "{plugin_id}" - api_v3.py save_main_config: exception message was still leaking through the error response; replace with generic message (consistent with the rest of the CodeQL sweep in this PR) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-24 09:29:53 -04:00
ChuckandChuck	71584d4361	Feature/widget registry system (#190) ... * chore: Update basketball-scoreboard submodule for odds font fix * feat(widgets): Add widget registry system for plugin configuration forms - Create core widget registry system (registry.js, base-widget.js) - Extract existing widgets to separate modules: - file-upload.js: Image upload with drag-and-drop, preview, delete, scheduling - checkbox-group.js: Multi-select checkboxes for array fields - custom-feeds.js: Table-based RSS feed editor with logo uploads - Implement plugin widget loading system (plugin-loader.js) - Add comprehensive documentation (widget-guide.md, README.md) - Include example custom widget (example-color-picker.js) - Maintain backwards compatibility with existing plugins - All widget handlers available globally for existing functionality This enables: - Reusable UI components for plugin configuration forms - Third-party plugins to create custom widgets without modifying LEDMatrix - Modular widget architecture for future enhancements Existing plugins (odds-ticker, static-image, news) continue to work without changes. * fix(widgets): Security and correctness fixes for widget system - base-widget.js: Fix escapeHtml to always escape (coerce to string first) - base-widget.js: Add sanitizeId helper for safe DOM ID usage - base-widget.js: Use DOM APIs in showError instead of innerHTML - checkbox-group.js: Normalize types in setValue for consistent comparison - custom-feeds.js: Implement setValue with full row creation logic - example-color-picker.js: Validate hex colors before using in style attributes - file-upload.js: Replace innerHTML with DOM creation to prevent XSS - file-upload.js: Preserve open schedule editors when updating image list - file-upload.js: Normalize types when filtering deleted files - file-upload.js: Sanitize imageId in openImageSchedule and all schedule handlers - file-upload.js: Fix max-files check order and use allowed_types from config - README.md: Add security guidance for ID sanitization in examples * fix(widgets): Additional security and error handling improvements - scripts/update_plugin_repos.py: Add explicit UTF-8 encoding and proper error handling for file operations - scripts/update_plugin_repos.py: Fix git fetch/pull error handling with returncode checks and specific exception types - base-widget.js: Guard notify method against undefined/null type parameter - file-upload.js: Remove inline handlers from schedule template, use addEventListener with data attributes - file-upload.js: Update hideUploadProgress to show dynamic file types from config instead of hardcoded list - README.md: Update Color Picker example to use sanitized fieldId throughout * fix(widgets): Update Slider example to use sanitized fieldId - Add sanitizeId helper to Slider example render, getValue, and setValue methods - Use sanitizedFieldId for all DOM IDs and query selectors - Maintain consistency with Color Picker example pattern * fix(plugins_manager): Move configurePlugin and togglePlugin to top of file - Move configurePlugin and togglePlugin definitions to top level (after uninstallPlugin) - Ensures these critical functions are available immediately when script loads - Fixes 'Critical functions not available after 20 attempts' error - Functions are now defined before any HTML rendering checks * fix(plugins_manager): Fix checkbox state saving using querySelector - Add escapeCssSelector helper function for safe CSS selector usage - Replace form.elements[actualKey] with form.querySelector for boolean fields - Properly handle checkbox checked state using element.checked property - Fix both schema-based and schema-less boolean field processing - Ensures checkboxes with dot notation names (nested fields) work correctly Fixes issue where checkbox states were not properly saved when field names use dot notation (e.g., 'display.scroll_enabled'). The form.elements collection doesn't reliably handle dot notation in bracket notation access. * fix(base.html): Fix form element lookup for dot notation field names - Add escapeCssSelector helper function (both as method and standalone) - Replace form.elements[key] with form.querySelector for element type detection - Fixes element lookup failures when field names use dot notation - Ensures checkbox and multi-select skipping logic works correctly - Applies fix to both Alpine.js method and standalone function This complements the fix in plugins_manager.js to ensure all form element lookups handle nested field names (e.g., 'display.scroll_enabled') reliably across the entire web interface. * fix(plugins_manager): Add race condition protection to togglePlugin - Initialize window._pluginToggleRequests map for per-plugin request tokens - Generate unique token for each toggle request to track in-flight requests - Disable checkbox and wrapper UI during request to prevent overlapping toggles - Add visual feedback with opacity and pointer-events-none classes - Verify token matches before applying response updates (both success and error) - Ignore out-of-order responses to preserve latest user intent - Clear token and re-enable UI after request completes Prevents race conditions when users rapidly toggle plugins, ensuring only the latest toggle request's response affects the UI state. * refactor(escapeCssSelector): Use CSS.escape() for better selector safety - Prefer CSS.escape() when available for proper CSS selector escaping - Handles edge cases: unicode characters, leading digits, and spec compliance - Keep regex-based fallback for older browsers without CSS.escape support - Update all three instances: plugins_manager.js and both in base.html CSS.escape() is the standard API for escaping CSS selectors and provides more robust handling than custom regex, especially for unicode and edge cases. * fix(plugins_manager): Fix syntax error - missing closing brace for file-upload if block - Add missing closing brace before else-if for checkbox-group widget - Fixes 'Unexpected token else' error at line 3138 - The if block for file-upload widget (line 3034) was missing its closing brace - Now properly structured: if (file-upload) { ... } else if (checkbox-group) { ... } * fix(plugins_manager): Fix indentation in file-upload widget if block - Properly indent all code inside the file-upload if block - Fix template string closing brace indentation - Ensures proper structure: if (file-upload) { ... } else if (checkbox-group) { ... } - Resolves syntax error at line 3138 * fix(plugins_manager): Skip checkbox-group [] inputs to prevent config leakage - Add skip logic for keys ending with '[]' in handlePluginConfigSubmit - Prevents checkbox-group bracket notation inputs from leaking into config - Checkbox-group widgets emit name="...[]" checkboxes plus a _data JSON field - The _data field is already processed correctly, so [] inputs are redundant - Prevents schema validation failures and extra config keys The checkbox-group widget creates: 1. Individual checkboxes with name="fullKey[]" (now skipped) 2. Hidden input with name="fullKey_data" containing JSON array (processed) 3. Sentinel hidden input with name="fullKey[]" and empty value (now skipped) * fix(plugins_manager): Normalize string booleans when checkbox input is missing - Fix boolean field processing to properly normalize string booleans in fallback path - Prevents "false"/"0" from being coerced to true when checkbox element is missing - Handles common string boolean representations: 'true', 'false', '1', '0', 'on', 'off' - Applies to both schema-based (lines 2386-2400) and schema-less (lines 2423-2433) paths When a checkbox element cannot be found, the fallback logic now: 1. Checks if value is a string and normalizes known boolean representations 2. Treats undefined/null as false 3. Coerces other types to boolean using Boolean() This ensures string values like "false" or "0" are correctly converted to false instead of being treated as truthy non-empty strings. * fix(base.html): Improve escapeCssSelector fallback to match CSS.escape behavior - Handle leading digits by converting to hex escape (e.g., '1' -> '\0031 ') - Handle leading whitespace by converting to hex escape (e.g., ' ' -> '\0020 ') - Escape internal spaces as '\ ' (preserving space in hex escapes) - Ensures trailing space after hex escapes per CSS spec - Applies to both Alpine.js method and standalone function The fallback now better matches CSS.escape() behavior for older browsers: 1. Escapes leading digits (0-9) as hex escapes with trailing space 2. Escapes leading whitespace as hex escapes with trailing space 3. Escapes all special characters as before 4. Escapes internal spaces while preserving hex escape format This prevents selector injection issues with field names starting with digits or whitespace, matching the standard CSS.escape() API behavior. --------- Co-authored-by: Chuck <chuck@example.com>	2026-01-16 14:09:38 -05:00