3 Commits
Author SHA1 Message Date
ChuckandClaude Opus 5.5 2236ff3081 fix(web-ui): MQTT password without TLS, Overview poll that never stopped, brightness slider error, token form left dirty (#745)
* fix(web-ui): let the MQTT bridge form save a password without TLS

PUT /api/v3/integrations/mqtt-bridge/config refuses a stored password
while mqtt_tls is off unless allow_insecure_mqtt is set (the CWE-319
guard in api_v3/misc.py). The Tools tab form neither rendered a control
for that flag nor sent it, so a password-protected broker on a LAN
without TLS could never be saved from the UI, and once such a password
was in bridge_config.json every later save from the form was refused.

The form now shows "Allow without TLS (trusted network)" while "Use
TLS" is unchecked, prefilled from the GET's config.allow_insecure_mqtt,
and mqttBody() sends its state as allow_insecure_mqtt. The box is off
until the user ticks it, so the server's guard still refuses a
cleartext password by default.

Tests: the Tools DOM suite checks the control, its show/hide with the
TLS box, the prefill and the value saved; a Flask test pins that the
GET reports the opt-in (false until saved on).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(web-ui): stop the Overview reconciliation poll from running forever

The reconciliation banner script in partials/overview.html re-asked
/api/v3/plugins/reconciliation-status every 2 s until the answer said
done, with no limit. The route answers done: false whenever
ledmatrix_reconciliation.json is missing or unreadable, which happens
when _run_startup_reconciliation raises before writing it or when /tmp
is cleaned under a long-running web service (reconciliation runs once
per process). The browser then sent that request every 2 s for as long
as the page stayed open, on every tab, since the poll was never tied to
the Overview being visible.

The poll now gives up after 30 tries (a minute) and runs only while the
Overview is the active, visible tab, registered with LEDVisibility under
its own key like the other partials' pollers. Dismissing the banner
ends it too.

Test: test/js/unit/test_overview_reconciliation_poll.js runs the shipped
script in a vm with fake timers and fetch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(web-ui): drop the Display tab's lookup of a removed brightness label

The brightness slider's input handler in partials/display.html set the
text of both #brightness-value and #brightness-display. #387
(978a03b42) removed the "LED brightness: N%" line that carried
#brightness-display, so getElementById returned null and every step of
the slider threw "Cannot set properties of null" into the console. The
visible label still updated, because it is written first.

The dead lookup is removed.

Test: test/js/unit/test_display_partial_ids.js checks every literal
getElementById() in the partial's inline scripts against the ids its
markup renders, and runs the shipped script in a vm to move the slider.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(web-ui): a created API token leaves the General tab's form clean

app.js marks a form data-dirty on any input inside it and removes the
mark only after a successful htmx request; its beforeunload handler
asks "Leave site?" while a visible form is still dirty. The API token
form in partials/general.html posts through window.webLogin.createToken
with fetch, so the mark survived the token being created and a reload
of the page with the General tab open prompted about a change that had
already been saved.

createToken now removes data-dirty after a successful create, next to
the form.reset() it already did. A refused request keeps the mark.

Test: test/js/unit/test_general_web_login_token.js runs the shipped
script in a vm with a fake fetch and DOM.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(js): match <script> tags the way CodeQL's tag-filter rule expects

The three new suites pull the inline scripts out of their partials with
/<script>([\s\S]*?)<\/script>/g. CodeQL flags that shape as a bad HTML
filtering regexp (js/bad-tag-filter: misses upper case and tags with
attributes or whitespace), four high alerts that blocked the PR. These are
our own templates read by tests, not user input, but the stricter pattern
costs nothing: /<script\b[^>]*>(...)<\/script[^>]*>/gi, as
test_html_escaping.js already uses.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(js): slice the Display partial's markup around its scripts

CodeQL read the script-stripping replace() as an incomplete HTML sanitizer
(js/incomplete-multi-character-sanitization). The test only reads our own
template, but slicing between the matched blocks gives the same markup
without the pattern.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 22:30:51 -04:00
ChuckandClaude Opus 5.5 e6e0a16140 ci: run the web UI DOM test suites; fix two stale suites (#660)
A new "Web UI JS tests" job installs jsdom, starts the web interface in
emulator mode and runs test/js/run_all.js with REQUIRE_DOM=1, which makes a
DOM suite that can't run a failure rather than a silent skip. (The unit
suites were already covered through pytest.)

Two suites failed against main when run for real:
- test_tools_sections rendered the Tools partial without LEDEscape, which
  base.html's app-early.js defines; it now installs it in beforeParse, and
  supplies two sample Starlark apps (one id with a quote) when the server
  has none, instead of assuming a device with apps and Pixlet.
- test_store_dom assumed the live registry had at most 48 plugins; it now
  checks pagination whichever side of 48 it is.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-28 15:01:08 -04:00
ChuckandClaude Opus 5 5137e86d16 feat(tools): MQTT bridge and Pixlet editor, ported onto the api_v3 split (#554)
* feat(tools): manage the MQTT bridge and Pixlet editor from the Tools tab

PR #544's change, ported onto the api_v3 package split (#553). Identical
behaviour; only the placement of the new code differs.

The original added 508 lines to web_interface/blueprints/api_v3.py, which #553
deletes, so every hunk of it would conflict irreconcilably. Ported by AST:
26 new top-level items sorted to where the split puts each kind --

  __init__.py   2 imports, 11 constants, 7 helpers
  starlark.py   4 routes  (/starlark/editor/{apps,status,start,stop})
  misc.py       2 routes  (/integrations/mqtt-bridge{,/config})

Everything outside api_v3.py -- the Tools partial, the installer scripts, the
JS tests -- applied unchanged.

Routes: 111 from the split plus these 6 = 117, and the url-map snapshot is
regenerated to match, which is exactly what test_api_v3_url_map.py is designed
to make you do when routes are added.

Full Python suite: 4,278 passed, 68 skipped, 0 failed. The JS tests this PR
ships could not be run here -- node is not installed on this machine -- so
test/js/dom/test_tools_sections.js is unverified.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

* fix(starlark): don't crash the pixlet editor's start/stop routes, and honor an operator-set PIXLET_EDITOR_HOST

The AST-based port of #544 onto the api_v3 package split dropped `time`
from starlark.py's import list. start_pixlet_editor() and
stop_pixlet_editor() both call time.time()/time.sleep() directly, so
every start (NameError building `state['started_at']`) and every stop
that has to wait out the EXIT trap crashed with a 500. No test caught
it because the route's own tests mock subprocess.Popen but never
actually invoked it before now.

Also carries over #544's later fix that this port branched before:
env['PIXLET_EDITOR_HOST'] = '0.0.0.0' unconditionally overrode an
operator who had already pinned PIXLET_EDITOR_HOST to loopback,
forcing the unauthenticated `pixlet serve` process onto the LAN
regardless (CodeQL CWE-1188). Switched to env.setdefault(...), same as
api_v3.starlark.py's siblings already do for _pkg-owned names.

Both fixes route the shared _pkg.time reference the rest of the
package's route modules already use for anything a test might need to
patch, rather than a bare `import time` local to this file.

Ported the existing regression test from #544
(TestPixletEditorHostDefaultsButDoesNotOverride) onto this branch's
module layout (web_interface.blueprints.api_v3.starlark instead of the
old monolithic api_v3 module), which is what caught the NameError.

Full suite: 4330 passed, 62 skipped, 2 failed -- identical on this
branch and on origin/main (missing tzdata package breaks two
timezone-alias tests in test_onboarding_checklist.py, unrelated to
this change).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(api-v3): clear the six lint errors this rebase introduced

All six were introduced by rebasing this branch onto the merged blueprint
split, not by the split itself. Confirmed by diffing pyflakes output against
main with line numbers normalised -- everything else it reports is present on
main too and is the package's deliberate re-export pattern.

starlark.py used _STARLARK_APPS_DIR three times without importing it (F821).
The rebase resolved an import-list conflict as a union of both sides, and that
symbol was on neither side of the conflict hunk, so it was silently lost. It is
defined in __init__.py and is now imported like its neighbours. This was the
only one of the six that would fail at runtime rather than merely lint.

__init__.py imported contextlib twice (F811): the cherry-pick added one next to
the existing import. Removed the duplicate; the original at line 19 is used.

__init__.py imported signal purely to re-export it to starlark.py, so pyflakes
saw it as unused (F401). signal is stdlib and does not need routing through the
blueprint package, so starlark.py imports it directly and __init__.py no longer
does. contextlib stays re-exported because this module genuinely uses it.

_read_mqtt_bridge_config()'s local `config` shadowed the `config` submodule
this module imports at the bottom for its route side effects (F811). Renamed to
`settings`, with a comment saying why, since the name is otherwise the obvious
one to reach for.

Verified: pyflakes now reports nothing on this branch that main does not, the
package imports, all nine route modules load, and 117 routes register, matching
the pinned URL-map snapshot.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(api-v3): reject MQTT bridge bodies the endpoint cannot apply

Two CodeRabbit findings on the bridge settings endpoint, both of which returned
200 while doing something other than what the caller asked.

`request.get_json(silent=True) or {}` turned a missing or unparseable body --
and the JSON literals null, [] and false -- into an empty dict, which then
satisfied the isinstance(data, dict) guard on the very next line. The guard was
there to reject exactly those bodies. Dropping the `or {}` lets None fail it.

The same `or {}` on /errors/clear is left alone: its docstring documents the
body as optional, so an absent body legitimately means "use the defaults". The
difference is that saving settings has nothing sensible to do with no body.

`if data.get('clear_password'):` accepted any truthy value, and the string
"false" is truthy in Python -- so a client echoing the field back as a string
wiped a password it meant to keep. Now coerced through the package's existing
_coerce_to_bool, which already maps 'true'/'on'/'1'/'yes' and nothing else.

test_mqtt_bridge_config_endpoint.py covers both: five unusable body shapes plus
a missing body, and clear_password across truthy and falsy spellings. Verified
against the unfixed code -- reverting the body guard fails 5, reverting the
coercion fails 3.

Not changed here: CodeRabbit also asks this endpoint to reject MQTT credentials
when TLS is off (CWE-319). That is a policy decision about the feature rather
than a defect -- unencrypted MQTT on a trusted LAN is common and often
deliberate -- so it is raised on the PR for a maintainer call instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: work through the remaining review findings on the editor and bridge

allow_insecure_mqtt (CWE-319, requested): a password with TLS disabled crosses
the network in cleartext. Refused now rather than merely warned about -- but
refused, not forbidden, because unencrypted MQTT on a trusted LAN is a normal
deliberate setup. allow_insecure_mqtt is the explicit acknowledgement, defaults
false, and is coerced like the other booleans so the string "false" cannot
switch the guard off.

starlark.py:796 -- the supported service runs Flask threaded, so two start
requests could each see running=False, each launch an editor, and the second
state write replace the first PID, orphaning a process that holds the display
down with nothing recording it. The check-launch-write sequence now takes a
module-level lock.

starlark.py:848 -- if the state write failed the route returned success with an
editor running and no PID recorded: status and stop both reported no session
while the display stayed down until the timeout expired. It now terminates the
process group and returns an error.

starlark.py:890 -- SIGKILL gives the script's EXIT trap no chance to run, so
nothing hands the display back, yet the response said "the display is
restarting". After an escalation the display is now restarted explicitly, and a
failure to do so returns an error naming the manual step instead of a success.

pixlet_config_editor.sh:184 -- find_pixlet supports Darwin but macOS ships no
timeout(1); GNU coreutils installs it as gtimeout. Resolved up front so the
failure lands before the display is stopped rather than after.

pixlet_config_editor.sh:154 -- wildcard, loopback and an explicit interface
address are three cases, not two. Collapsing the last two printed a URL saying
"localhost" whenever PIXLET_EDITOR_HOST named a LAN address.

tools.html:1254 -- escHtml does not encode single quotes, and the app id was
interpolated into an inline onclick="startPixletEditor('...')", so a directory
containing an apostrophe could break out of the JS string and run script. The
handler binds with addEventListener and reads the id from dataset, where it is
only ever parsed as an HTML attribute.

Tests: test_mqtt_bridge_config_endpoint.py grows to 23 cases covering the opt-in
in both directions. The tools DOM suite gains three guards asserting the edit
buttons carry no inline onclick and pass the id via dataset -- those need jsdom
and did not run here, so CI verifies them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(api-v3): log the traceback on the editor state-write failure

The 848 fix answers 500 when the session state cannot be written, and logged
that at error level -- but without exc_info, so the traceback never reached the
log. test_web_error_detail.py guards exactly this: a handler returning 5xx must
write an error-level record *with* the traceback and return the sanitized
detail, because checking that merely something was logged is too weak.

Caught by Core unit tests on the previous commit, not locally: the guard parses
every module under web_interface/blueprints/api_v3 as one source, so it only
fires once the whole package is read together.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-11 11:32:12 -04:00