Files
LEDMatrix/test/test_composer_empty_block.py
Claude 7603728e5a fix(composer): close binding_source code injection, line-align, and project_root UnboundLocalError
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>
2026-09-12 20:43:16 +00:00

125 lines
5.1 KiB
Python

"""An element the template cannot draw must not produce an empty `if` block.
manager.py.j2 wraps each element in `if width >= N:` (breakpoint) and/or
`if int(time.time() * 2) % 2:` (blink), and the body comes from the per-type
branches. A type with no branch contributed nothing, so the wrapper opened a
block with no statements in it. ast.parse in _generate_plugin_files then
failed and the caller was told only:
Generated code has a syntax error: expected an indented block after
'if' statement on line 49
which names a line of generated source the user never sees. Confirmed against
the code before the fix with a `group` element carrying minWidth.
Two defences, both covered here: _preprocess_elements drops types the template
has no branch for, and the template emits a `pass` fallback so a type added to
the canvas before its branch exists degrades to a no-op instead of a broken
plugin.
"""
import ast
import re
import sys
from pathlib import Path
import pytest
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from web_interface.blueprints import composer as C # noqa: E402
TEMPLATE = (Path(__file__).resolve().parent.parent
/ "web_interface/templates/v3/composer/manager.py.j2")
BASE_META = {"id": "test-plugin", "name": "Clock", "author": "a",
"version": "1.0.0", "description": "d"}
def generate(element):
return C._generate_plugin_files({
"metadata": BASE_META,
"elements": [element],
"dataModel": {"configVars": []},
})
@pytest.mark.parametrize("wrapper", [
{"minWidth": 64}, # breakpoint block
{"blink": True}, # blink block
{"minWidth": 64, "blink": True}, # both, nested
])
@pytest.mark.parametrize("etype", ["group", "widget_9000", "section"])
def test_undrawable_element_does_not_break_generation(etype, wrapper):
element = {"type": etype, "x": 0, "y": 0, "color": "#ffffff", **wrapper}
files = generate(element) # must not raise ComposerInputError
assert "manager.py" in files
def test_drawable_element_still_renders_inside_a_breakpoint():
files = generate({"type": "text", "text": "hi", "x": 0, "y": 0,
"minWidth": 64, "color": "#ffffff"})
src = files["manager.py"]
assert "if width >= 64:" in src
assert "draw_text" in src
def test_renderable_types_match_the_template_branches():
"""The constant and the template must agree.
A type listed in the constant with no branch emits an empty block (the bug
above); a type with a branch but missing from the constant is silently
dropped from every generated plugin. Neither is visible without this check.
"""
branches = set(re.findall(r"el\.type == '([a-z_]+)'", TEMPLATE.read_text()))
assert branches == set(C._RENDERABLE_ELEMENT_TYPES)
def test_template_closes_the_branch_chain_with_a_fallback():
"""Belt and braces: even if the constant drifts, no empty block escapes."""
text = TEMPLATE.read_text()
assert "{% else %}" in text
assert "pass # element type" in text
@pytest.mark.parametrize("wrapper", [
{"minWidth": 64},
{"blink": True},
{"minWidth": 64, "blink": True},
])
def test_dynamic_text_with_non_config_binding_does_not_break_generation(wrapper):
"""dynamic_text only renders a body for binding.source == 'config'.
_preprocess_elements accepts any string as binding.source (it just
defaults a missing one to 'config'), so a 'live' or 'sensor' source --
anything a client sends that isn't literally 'config' -- hit the same
empty-if-block bug as an undrawable element type, just one level deeper:
the branch is taken, but its own inner `if` produced nothing.
"""
element = {"type": "dynamic_text", "x": 0, "y": 0, "color": "#ffffff",
"binding": {"source": "live", "key": "temperature"}, **wrapper}
files = generate(element) # must not raise ComposerInputError
assert "manager.py" in files
assert "non-config dynamic_text binding draws nothing" in files["manager.py"]
def test_binding_source_cannot_break_out_of_the_comment_it_lands_in():
"""binding.source used to be interpolated straight into a Python comment
(`pass # dynamic_text binding_source "{{ el.binding_source }}" draws
nothing`) with no escaping. A source string carrying a newline closed the
comment, and text on the following line(s), indented to match, became a
real statement in the generated plugin -- CWE-94, and live once
composer_bp was registered (confirmed: this exact payload produces a
manager.py containing a clean, ast.parse-valid `import os` against the
pre-fix template). The comment is now a fixed literal that never
interpolates the value at all.
"""
payload = "foo\n import os\n os.system('id') # "
element = {"type": "dynamic_text", "x": 0, "y": 0, "color": "#ffffff",
"binding": {"source": payload, "key": "temperature"}}
files = generate(element)
src = files["manager.py"]
assert "import os" not in src
assert "os.system" not in src
assert "non-config dynamic_text binding draws nothing" in src
ast.parse(src) # belt and braces: generate() already enforces this