4 Commits

Include renames

Author	SHA1	Message	Date
Claude	324a7eac87	fix(web): style editor no longer strands leaf-valued layout fields ... A prior fix on this PR made elementKeys() append any layout-only key with no style block of its own (a logo, a timeout indicator, a possession arrow), so table() draws a row for it instead of losing it when the wholesale `layout` claim removes the generic fallback. That covers a layout-only key shaped like an object (x_offset/y_offset, ...), because columnsFor() only ever produced columns from a key's *sub-fields*. It missed the case where the layout-only key's own value is itself a leaf -- a plain "show_logo" boolean directly under layout, no x/y object underneath. elementKeys() still lists it (any row: no matching column), so it renders as an uneditable blank row and its only control -- the generic fallback checkbox -- is still gone. Confirmed by executing the real widget's render() against a synthetic schema in Node (a DOM-stub harness, not committed): the field's name never appeared as an <input>. columnsFor() now gives such a leaf key a column keyed to itself ('layout-leaf'), and elementRow() binds it to the leaf's own path (customization.layout.<key>, matching the name the fallback would have used) instead of leaving every cell blank. New regression test (test/js/unit/test_style_editor_layout_leaf_columns.js, following this PR's existing eval-extraction pattern) checks the leaf column is produced, is self-keyed, doesn't duplicate, and that a schema with no leaf-valued layout key is unaffected; wired into run_all.js and the suite table in test/js/README.md. test/js/run_all.js: 84 + 6 + 6 = all suites passed (jsdom unavailable here, DOM suites skip as before). Python suite untouched by this change; test_style_editor_extra_fields.py, test_style_editor_save_roundtrip.py and the one PIL-dependent style_editor_takeover.py case fail identically before this commit -- missing flask/PIL in this sandbox, not this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-14 03:36:14 +00:00
Claude	75e6b2b20c	fix(web): style editor no longer drops layout-only fields it never rendered ... CodeRabbit flagged elementKeys() in style-editor.js: render() claims the whole customization.layout child as the widget's own (removing it from the generic fallback renderer, since posting the same offset twice is worse), but elementKeys() only listed keys that also have their own top-level style block. A hand-written schema can put a key under layout that never got one -- a logo, a timeout indicator, a possession arrow with a position but no font or colour -- and that key's only control silently disappeared: no row in the style editor's table (elementKeys never listed it) and no fallback section either (layout was removed wholesale). elementKeys() now appends any layout-declared key not already covered by a style element, so table() renders a row for it (layout columns only, no style columns) and the wholesale layout ownership claim stays truthful. Verified against current code before fixing. New regression test (test/js/unit/test_style_editor_element_keys.js, following this repo's existing eval-extraction pattern for testing widget JS without a browser) fails against the reverted function and passes with the fix; added to run_all.js and the suite table in test/js/README.md. Full pytest suite: 4887 passed, 62 skipped, 2 failed -- both the pre-existing Europe/Kiev/Asia/Calcutta tzdata-alias gap on this sandbox, identical on origin/main, unrelated to this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dpg3HLWohdCUdzz2QNHanm	2026-09-13 23:38:37 +00: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
ChuckandClaude Opus 5	4423ec33d5	feat(plugins): search, filter and sort for Installed Plugins, on a shared ListFilter helper (#540) ... * feat(plugins): add search, filter and sort to Installed Plugins, on a shared helper The Installed Plugins grid had no way to narrow it down: no search, no way to see only what's enabled, disabled, or out of date. On a rig with a couple dozen plugins that means scrolling the whole grid to find one. The two sections below it already solved this, twice, independently — the Plugin Store and Starlark Apps carried a copy-paste fork of the same ~600 lines (filter state, apply-filters-and-sort, page renderer, pagination strip, active-filter badge, listener wiring). Rather than add a third copy, this extracts the shared machinery and builds the new toolbar on it. New: web_interface/static/v3/js/plugins/list_filter.js — ListFilter.create() owns debounced search, filter axes, sort, the active-filter count, Clear, and optional pagination/persistence. Callers keep their own card markup via a `render` callback. Three control types cover every axis the page uses: pills (new), select (store category, starlark author) and cycle (the tri-state All -> Installed -> Not Installed button). Installed Plugins gets a compact toolbar: search box, one-click All / Enabled / Disabled / Updates pills, and a sort dropdown (A-Z, Z-A, updates first, recently updated, category). Filters reset on load, so you never come back to a mysteriously short list. No new CSS — this is the first consumer of the .filter-pill rules already sitting unused in app.css. renderInstalledPlugins() is split so it still publishes canonical state while renderInstalledCards() draws only the visible subset; the filtered list is never assigned to window.installedPlugins, which the toggle handler, isStorePluginInstalled(), runUpdateAllPlugins() and the Alpine config tabs all read as their source of truth. Toggling a plugin while filtered pins its card so it doesn't vanish from under the cursor. The Store and Starlark migrations are behaviour-preserving: same element ids, same localStorage keys (storeSort/storePerPage, starlarkSort/starlarkPerPage), same tri-state button markup, same pagination. Verified by differential tests that run the old and new implementations side by side against identical fixtures and compare every observable after each interaction. The only visible change is the pagination attribute (data-store-page/data-starlark-page -> data-list-page), which nothing outside its own click handler referenced. Net -156 lines in plugins_manager.js while adding a feature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * fix(plugins): keep raw search text, and stop the store search refetching Two review findings from CodeRabbit on #540. Do not write the trimmed search value back into the input. setSearch() trimmed before storing, and syncControls() then copied that trimmed value back over what the user had typed. Pausing longer than the debounce after typing a space deleted the space (and reset the caret), making multi-word terms effectively untypable. The raw text is now kept alongside the trimmed one: filtering and activeCount() still use the trimmed value, while the input keeps exactly what was typed. Remove the legacy #plugin-search / #plugin-category listeners in initializePlugins(). They bound searchPluginStore as the event handler, so the DOM event arrived as its `fetchCommitInfo` argument — always truthy, which skipped the cached-filter fast path and refetched /api/v3/plugins/store/list with commit info on every keystroke burst and category change. The store's ListFilter controller already filters the cached list, which is what those two controls should do. This double-binding predates this PR (the old code guarded with _listenerSetup and _storeFilterInit, two different flags, so both sets stayed live); it is fixed here because the refactor owns that wiring now. Both fixes are covered by tests that fail without them: the trailing-space regressions in the installed-plugins DOM suite, and a new whole-file jsdom test that counts fetches while typing (1 request at init, 0 thereafter; previously 1 -> 2 -> 3 -> 5). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * fix(plugins): build pagination via DOM APIs, drop computed member access Addresses the five Codacy security findings, all in list_filter.js. Pagination no longer assembles an HTML string (3 findings: 2 critical + 1 high, "unsafe assignment to innerHTML"). The interpolated values were only page integers and local class constants, so there was no injection path, but concatenating markup into innerHTML is the pattern the scanners flag and createElement is no less clear. Each button now also owns its click listener directly instead of the container being re-queried afterwards, and the strip is cleared with textContent = '' rather than by assigning empty markup. No innerHTML assignment remains in the file. haystack() now walks Object.entries(item) and keeps the configured fields, instead of reading item[field] per field ("generic object injection sink"). Field order no longer drives the haystack order, which is irrelevant to the substring test. matches() iterates controls with for...of instead of an index ("variable assigned to object injection sink"). The rendered pagination is unchanged: same buttons, labels, page numbers, disabled states and classes. The old-vs-new differential tests now compare pagination structurally (tag, text, page, disabled, sorted class list) rather than as an HTML string, since building nodes legitimately serialises differently — «/» as characters rather than &laquo;/&raquo;, disabled="" rather than a bare attribute. That comparison is stronger than the string one it replaces, and the real-DOM suite still drives the actual page buttons. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * fix(plugins): keep configured field order when building the search haystack The previous commit swapped item[field] for Object.entries(item) to clear a static-analysis object-injection warning, and in doing so changed the order of the haystack: entries follow the object's own key insertion order, not the configured `fields` order. Since the values are concatenated, that order decides which values end up adjacent, so a multi-word query spanning a field boundary matched differently. For store fields [name, description, author, id, ...] and API objects keyed {id, name, description, author, ...}, "bob plugin-01" matched before and stopped matching after. That contradicted the behaviour-preservation claim for the store and starlark migrations, and the differential tests missed it because every fixture query was a single word. Values now come out of a Map built from Object.entries, iterated in `fields` order: the original haystack is restored, and there is still no computed member access for the analyser to flag. Regression coverage for the ordering itself, at both levels: - unit: phrases spanning name->id and category->tags, plus the reverse (object-key) order asserted NOT to match - differential: the same class of query compared old-vs-new, with a guard that the phrase actually matches something so a mutual zero-result cannot pass vacuously Verified both fail without this fix (3 unit, 2 differential) and pass with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * test(web): add JS suites for ListFilter and the plugin-manager grids No JS toolchain exists in this repo, so these are plain node scripts with no framework: each prints ok/FAIL lines and exits non-zero. `node test/js/run_all.js` runs everything, skipping the DOM suites (rather than failing) when jsdom is absent or nothing is listening, so it stays useful in a bare checkout. unit/test_list_filter.js ListFilter search/filter/sort/count/sticky, driven through the installed-plugins config eval'd verbatim out of plugins_manager.js so the test cannot drift from the real configuration unit/test_render_cards.js renderInstalledCards markup, both empty states, and escaping of hostile plugin metadata dom/test_installed_dom.js the toolbar in a real DOM, including the HTMX partial re-swap and a getComputedStyle check that .filter-pill[data-active] matches what we emit dom/test_store_dom.js store pagination, per-page, category, tri-state Installed button, persistence across a re-boot dom/test_no_double_fetch.js loads the whole plugins_manager.js and counts requests, so a keystroke cannot refetch the store The DOM suites deliberately fetch the partial and the plugin data from a running web interface instead of using fixtures, so a renamed element id or a changed payload shape fails them loudly. Point them at a rig with a full plugin set when it matters (BASE=http://host:5000); a dev box with two plugins installed passes while exercising very little. Several assertions exist to stop specific bugs recurring: trailing spaces surviving the search debounce, a query spanning two adjacent search fields (haystack field order is load-bearing), and window.installedPlugins staying at full length while the grid is filtered. Others guard against passing vacuously — counting only non-skeleton cards, and checking a search phrase matches something before comparing two result sets. The old-vs-new differential suites that verified the store and starlark migrations are not included: they compared against the pre-refactor code, which now exists only in git history. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-08 20:04:47 -04:00