From 98d4b3b55b38de38277398db89a6edd593029764 Mon Sep 17 00:00:00 2001 From: Chuck Date: Sat, 30 May 2026 19:59:57 -0400 Subject: [PATCH] fix(security): address CodeQL and coderabbit review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Security fixes ### pages_v3.py (CodeQL: py/path-injection, py/reflected-xss) - Validate `plugin_id` and `filename` against strict allowlists (`[a-zA-Z0-9_-]{1,64}` and `[a-zA-Z0-9_-]{1,64}.html`) before any path or script operations — satisfies CodeQL path-injection checks - Error responses returned as `text/plain` with no user data in body - HTML-meta-char escaping on PLUGIN_ID value in script tag (defence in depth) ### array-table.js (CodeQL: js/prototype-pollution) - Guard `setNestedValue()` against `__proto__`, `prototype`, and `constructor` keys; silently drops any write targeting those keys ### plugin-file-manager.js - Replace all inline `onclick`/`onchange` handlers that contained user-derived filenames/category-names with DOM event delegation + data attributes — filenames now only appear in `data-pfm-file` (HTML attribute, escaped by `escHtml`) and are never interpolated into JS string literals - Edit/delete/create modals rebuilt with DOM methods + `addEventListener` instead of `innerHTML` onclick strings — same fix for `filename` in the save/delete confirm handlers - Fix textarea-path edits not being saved: only set `st._editData` for the tabular code path; leave it null for the textarea path so `_pfmSave()` reads ` -
`; +
`; } }; @@ -401,14 +431,23 @@ st._tableCols = cols; } + // Store buildPage in per-instance state so multiple instances don't + // clobber each other's pagination via a shared global. + st._buildPage = buildPage; buildPage(st._tablePage || 1); - window._pfmTablePage = function (fId, p) { - const s = getState(fId); - const totalP = Math.ceil(s._tableEntries.length / s.entriesPerPage); - buildPage(Math.max(1, Math.min(p, totalP))); - }; } + // Global dispatcher — resolves the per-instance buildPage from state so + // multiple plugin-file-manager instances don't clobber each other. + window._pfmTablePage = function (fId, p) { + const s = getState(fId); + if (s._buildPage) { + const total = s._tableEntries ? s._tableEntries.length : 0; + const totalP = Math.ceil(total / s.entriesPerPage) || 1; + s._buildPage(Math.max(1, Math.min(p, totalP))); + } + }; + window._pfmCellEdit = function (fieldId, day, col, value) { const st = getState(fieldId); if (st._editData && st._editData[day]) st._editData[day][col] = value; @@ -454,30 +493,32 @@ window._pfmOpenDelete = function (fieldId, filename) { const overlay = createOverlay(fieldId); - overlay.innerHTML = ` -
-
- Delete File - -
-
-
- ${escHtml(filename)} will be permanently deleted and removed - from the plugin configuration. This cannot be undone. -
-
-
- - + const modal = document.createElement('div'); + modal.className = 'pfm-modal'; + modal.style.maxWidth = '28rem'; + modal.innerHTML = ` +
+ Delete File + +
+
+
+ ${escHtml(filename)} will be permanently deleted and removed + from the plugin configuration. This cannot be undone.
+
+
+ +
`; + overlay.appendChild(modal); + modal.querySelector(`#${CSS.escape(fieldId)}_del_close`).addEventListener('click', () => window._pfmCloseModal(fieldId)); + modal.querySelector(`#${CSS.escape(fieldId)}_del_cancel`).addEventListener('click', () => window._pfmCloseModal(fieldId)); + modal.querySelector(`#${CSS.escape(fieldId)}_del_confirm`).addEventListener('click', () => window._pfmConfirmDelete(fieldId, filename)); }; window._pfmConfirmDelete = async function (fieldId, filename) { @@ -499,35 +540,38 @@ const st = getState(fieldId); const fields = st.createFields; const overlay = createOverlay(fieldId); - overlay.innerHTML = ` -
-
- Create New File - -
-
-
- ${fields.map(f => ` -
- - - ${f.hint ? `
${escHtml(f.hint)}
` : ''} -
`).join('')} -
-
- - -
+ const modal = document.createElement('div'); + modal.className = 'pfm-modal'; + modal.style.maxWidth = '32rem'; + modal.innerHTML = ` +
+ Create New File + +
+
+
+ ${fields.map(f => ` +
+ + + ${f.hint ? `
${escHtml(f.hint)}
` : ''} +
`).join('')} +
+
+ + +
`; + overlay.appendChild(modal); + modal.querySelector(`#${CSS.escape(fieldId)}_cre_close`).addEventListener('click', () => window._pfmCloseModal(fieldId)); + modal.querySelector(`#${CSS.escape(fieldId)}_cre_cancel`).addEventListener('click', () => window._pfmCloseModal(fieldId)); + modal.querySelector(`#${CSS.escape(fieldId)}_create_btn`).addEventListener('click', () => window._pfmConfirmCreate(fieldId)); }; window._pfmConfirmCreate = async function (fieldId) { diff --git a/web_interface/static/v3/js/widgets/time-picker.js b/web_interface/static/v3/js/widgets/time-picker.js index 613a15d6..6752334d 100644 --- a/web_interface/static/v3/js/widgets/time-picker.js +++ b/web_interface/static/v3/js/widgets/time-picker.js @@ -148,6 +148,7 @@ onClear: function(fieldId) { const widget = window.LEDMatrixWidgets.get('time-picker'); widget.setValue(fieldId, ''); + widget.validate(fieldId); // refresh required/error state triggerChange(fieldId, ''); } } diff --git a/web_interface/templates/v3/partials/plugin_config.html b/web_interface/templates/v3/partials/plugin_config.html index 6859e9c7..4d40483a 100644 --- a/web_interface/templates/v3/partials/plugin_config.html +++ b/web_interface/templates/v3/partials/plugin_config.html @@ -526,7 +526,7 @@ {% for col_name in display_columns %} {% set col_def = item_properties.get(col_name, {}) %} {% set col_type = col_def.get('type', 'string') %} - {% set col_xwidget = col_def.get('x-widget', '') %} + {% set col_xwidget = col_def.get('x-widget') or col_def.get('x_widget', '') %} {% set col_enum = col_def.get('enum', []) %} {% set col_value = item.get(col_name, col_def.get('default', '')) %} {% if col_xwidget == 'date-picker' %}{% set td_min_w = '140px' %} @@ -1033,7 +1033,7 @@ or if every action is marked ui_hidden in the manifest. #} {% set has_file_manager_widget = namespace(value=false) %} {% for _fk, _fp in schema.get('properties', {}).items() %} - {% if _fp.get('x-widget') in ('json-file-manager', 'plugin-file-manager') %} + {% if (_fp.get('x-widget') or _fp.get('x_widget')) in ('json-file-manager', 'plugin-file-manager') %} {% set has_file_manager_widget.value = true %} {% endif %} {% endfor %}