Commit Graph

2 Commits

Author SHA1 Message Date
Chuck
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
Chuck
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