mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 14:25:08 +00:00
Claude
7603728e5a
Three findings from CodeRabbit's review of 6c23994b, all verified against
current code before fixing:
- manager.py.j2 interpolated binding.source unescaped into a Python comment
(`pass # dynamic_text binding_source "{{ el.binding_source }}" draws
nothing`). A source string with a newline broke out of the comment; a
crafted payload produces a clean, ast.parse-valid `import os` in the
generated plugin (confirmed against the pre-fix template). This is now a
fixed literal comment that never interpolates the value. Live now that
composer_bp is registered. CWE-94.
- _alignElement moved a line's x0 (or y0) to the new position but left x1
(or y1) behind, so aligning a line changed its shape instead of moving
it. Both endpoints now translate by the same delta.
- web_interface/app.py only assigned project_root inside the relative-path
branch of the plugins_dir resolution. An absolute plugin_system.plugins_
directory (a supported config value) hit UnboundLocalError importing the
module at all, since SchemaManager/composer_bp use project_root further
down. Now assigned unconditionally before the branch.
Also extends BOUND_TYPES coverage in composer-app.js (_isBound,
removeConfigVar, _validateBeforeExport) from dynamic_text/progress_bar to
all six element types that carry a binding object (countdown, pips,
sparkline, gauge too) -- found by direct code reading against
ELEMENT_DEFAULTS in composer-canvas.js, not from a review comment. Without
it, those four types could export with an unbound config key with no
validation error, and deleting a config var they used gave no warning.
All four fixes have mutation-checked regression tests (fail against the
reverted code, pass with the fix): test_binding_source_cannot_break_out_of_the_comment_it_lands_in,
test_align_translates_both_line_endpoints_not_just_the_start,
test_app_plugins_dir_resolution.py, test_binding_checks_cover_every_bound_element_type.
Full suite: 4365 passed, 58 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>
53 lines
2.2 KiB
Python
53 lines
2.2 KiB
Python
"""web_interface/app.py resolves plugins_dir from config at import time.
|
|
|
|
`project_root` used to be assigned only inside the `else` branch (the
|
|
relative-path case). SchemaManager and composer_bp are wired up with
|
|
`project_root` further down the same module, so a device configured with an
|
|
*absolute* `plugin_system.plugins_directory` (a supported value -- see
|
|
CLAUDE.md) hit `UnboundLocalError` importing web_interface.app at all, taking
|
|
the whole web UI down.
|
|
|
|
app.py isn't imported directly by the test suite -- it has real side effects
|
|
(managers, background state) at import time -- so this extracts the exact
|
|
resolution block by its stable comment markers and executes it in isolation,
|
|
the same way the rest of the suite validates generated/templated code rather
|
|
than re-implementing it.
|
|
"""
|
|
from pathlib import Path
|
|
|
|
APP_PY = Path(__file__).resolve().parent.parent / "web_interface/app.py"
|
|
|
|
START_MARKER = "# Resolve plugin directory - handle both absolute and relative paths\n"
|
|
END_MARKER = "\nplugin_manager = PluginManager("
|
|
|
|
|
|
def _resolution_block() -> str:
|
|
src = APP_PY.read_text()
|
|
start = src.index(START_MARKER)
|
|
end = src.index(END_MARKER, start)
|
|
return src[start:end]
|
|
|
|
|
|
def test_resolution_block_still_matches_expected_markers():
|
|
"""If app.py is restructured enough that these markers move, the exec
|
|
below would silently test nothing -- fail loudly instead."""
|
|
block = _resolution_block()
|
|
assert "plugins_dir" in block and "os.path.isabs" in block
|
|
|
|
|
|
def test_absolute_plugins_directory_does_not_raise_unboundlocalerror():
|
|
import os
|
|
ns = {"os": os, "Path": Path, "__file__": str(APP_PY),
|
|
"plugins_dir_name": "/opt/ledmatrix-plugins"}
|
|
exec(compile(_resolution_block(), str(APP_PY), "exec"), ns) # noqa: S102
|
|
assert ns["project_root"] == APP_PY.parent.parent
|
|
assert ns["plugins_dir"] == Path("/opt/ledmatrix-plugins")
|
|
|
|
|
|
def test_relative_plugins_directory_still_resolves_under_project_root():
|
|
import os
|
|
ns = {"os": os, "Path": Path, "__file__": str(APP_PY),
|
|
"plugins_dir_name": "plugin-repos"}
|
|
exec(compile(_resolution_block(), str(APP_PY), "exec"), ns) # noqa: S102
|
|
assert ns["plugins_dir"] == ns["project_root"] / "plugin-repos"
|