fix(web-ui): MQTT password without TLS, Overview poll that never stopped, brightness slider error, token form left dirty by ChuckBuilds · Pull Request #745 · ChuckBuilds/LEDMatrix

ChuckBuilds · 2026-10-04T01:59:33Z

Summary

Four front-end fixes, one commit each.

The MQTT bridge form could not save a password without TLS.
- Cause: the server refuses a cleartext password unless allow_insecure_mqtt is set, and the form had no way to set it. Once a password was stored without TLS, every later save failed, even a log-level change.
- Fix: a new "Allow without TLS (trusted network)" box, shown only while Use TLS is off. It is prefilled from the saved config and unticked by default, so the server's protection is unchanged.
The Overview polled /plugins/reconciliation-status every 2 s forever when the status file was missing (reconciliation raised, or /tmp was cleaned).
- Fix: the poll gives up after 30 tries and runs only while the Overview is on screen (LEDVisibility, under its own key).
Moving the Display tab's brightness slider threw a TypeError on every input. The lookup pointed at an element Add settings tooltips and search to the web UI #387 removed; the dead line is gone.
Creating an API token on the General tab left the form marked dirty, so a reload asked "Leave site?". The mark is now cleared after a successful create.

Not fixed, needs an owner decision

The MQTT bridge Install / Start / Stop / Restart buttons on the Tools tab answer "Unknown action". Their handler branches existed in feat(tools): manage the MQTT bridge and Pixlet editor from the Tools tab #544 and were lost in the feat(tools): MQTT bridge and Pixlet editor, ported onto the api_v3 split #554 port.
Why they weren't restored: the web service runs as the install user. The sudoers file in scripts/install/lib_sudoers.sh only allows systemctl for ledmatrix and ledmatrix-web, and the install script needs sudo tee and daemon-reload.
What would make them work:
- three new sudoers rules for ledmatrix-mqtt-bridge.service;
- for Install, either a root-owned helper with its own rule, or keeping Install command-line only.
That needs your call and testing on Linux, so it is left out.

Tests

New JS units: test_overview_reconciliation_poll.js, test_display_partial_ids.js, test_general_web_login_token.js. Also an extended dom/test_tools_sections.js and a Flask test for the MQTT GET.
REQUIRE_DOM=1 node test/js/run_all.js against the live emulator web UI: 26 of 26 suites, all 9 DOM suites included.
Checked in a browser:
- the slider updates its label with no error;
- the allow box hides while TLS is on;
- saving a password with TLS off is refused (400) with the box unticked and saved (200, with a cleartext warning logged) with it ticked.

Part of a bug sweep

This is one of 10 independent fix PRs from one sweep, all based on main ef69201.

Merge order: any. 45 pairwise test merges gave 0 conflicts, and each PR also merges cleanly with fix(ipc): ticks carry the volatile timestamps, so current-status stays known over the socket #737.
CHANGELOG: each PR adds its bullet at a different place in Unreleased → Fixes, so squash-merging them one after another needs no conflict fixing.
Full suite (Windows), all 10 merged together vs plain main: the same 62 failures and 6 errors on both. These are the known Windows path and file-locking tests. 191 more tests pass.
- One extra failure in that run, test_backup_manager.py::test_create_backup_contents (os.replace → WinError 5 on a temp zip), was a Windows file-lock flake. It passes on rerun, and nothing here touches create_backup.
ledpi: all 10 together ran on ledpi (Pi 4) on top of main, with a clean start and no errors or render stalls in the journal. ledpi is back on plain main.

🤖 Generated with Claude Code

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>

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>

The brightness slider's input handler in partials/display.html set the text of both #brightness-value and #brightness-display. #387 (978a03b) 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>

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>

coderabbitai · 2026-10-04T01:59:43Z

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 1daf0b51-0fd6-421a-b88d-670ba02902d2

📥 Commits

Reviewing files that changed from the base of the PR and between ef69201 and 6d18696.

📒 Files selected for processing (12)

CHANGELOG.md
test/js/README.md
test/js/dom/test_tools_sections.js
test/js/run_all.js
test/js/unit/test_display_partial_ids.js
test/js/unit/test_general_web_login_token.js
test/js/unit/test_overview_reconciliation_poll.js
test/test_mqtt_bridge_config_endpoint.py
web_interface/templates/v3/partials/display.html
web_interface/templates/v3/partials/general.html
web_interface/templates/v3/partials/overview.html
web_interface/templates/v3/partials/tools.html

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codacy-production · 2026-10-04T02:03:13Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

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>

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>

ChuckBuilds and others added 4 commits October 3, 2026 20:50

github-advanced-security Bot found potential problems Oct 4, 2026

View reviewed changes

Comment thread test/js/unit/test_display_partial_ids.js Fixed

Comment thread test/js/unit/test_display_partial_ids.js Fixed

Comment thread test/js/unit/test_general_web_login_token.js Fixed

Comment thread test/js/unit/test_overview_reconciliation_poll.js Fixed

github-advanced-security Bot found potential problems Oct 4, 2026

View reviewed changes

Comment thread test/js/unit/test_display_partial_ids.js Fixed

ChuckBuilds merged commit 2236ff3 into main Oct 4, 2026
15 checks passed

ChuckBuilds deleted the fix/web-ui-tools-display-general branch October 4, 2026 02:30

Navigation Menu

Uh oh!

fix(web-ui): MQTT password without TLS, Overview poll that never stopped, brightness slider error, token form left dirty - #745

fix(web-ui): MQTT password without TLS, Overview poll that never stopped, brightness slider error, token form left dirty#745
ChuckBuilds merged 6 commits into
mainfrom
fix/web-ui-tools-display-general

ChuckBuilds commented Oct 4, 2026

Uh oh!

coderabbitai Bot commented Oct 4, 2026 •

edited

Loading

Review limit reached

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codacy-production Bot commented Oct 4, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ChuckBuilds commented Oct 4, 2026

Summary

Not fixed, needs an owner decision

Tests

Part of a bug sweep

Uh oh!

coderabbitai Bot commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codacy-production Bot commented Oct 4, 2026

Up to standards ✅

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Oct 4, 2026 •

edited

Loading