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>

test/test_app_plugins_dir_resolution.py

@@ -0,0 +1,52 @@
+"""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"

test/test_composer_blueprint_registered.py

@@ -0,0 +1,64 @@
+"""composer_bp must actually be registered on the Flask app.
+
+web_interface/blueprints/composer.py defines a full blueprint -- the
+/composer/ page, /composer/api/generate, /composer/api/install, etc. -- but
+nothing in web_interface/app.py imported or registered it. pages_v3 and
+api_v3 both go through the same "import, wire managers, register_blueprint"
+sequence; composer_bp had none of the three. The routes existed and worked
+in isolation (every other composer test builds its own minimal Flask app
+and registers composer_bp directly), but in the real running app every
+composer URL -- including the page itself -- was a 404, and the frontend's
+hardcoded '/composer/api/...' fetches (composer-app.js) had nothing to
+reach.
+
+app.py has import-time side effects (it loads config, builds managers, and
+in some configurations starts background threads), so this scans the source
+text rather than importing the module -- the same approach already used for
+test_composer_js_contracts.py's structural checks.
+"""
+import re
+from pathlib import Path
+
+APP_PY = Path(__file__).resolve().parent.parent / "web_interface" / "app.py"
+
+
+def _source() -> str:
+    return APP_PY.read_text()
+
+
+def test_composer_blueprint_is_imported():
+    src = _source()
+    assert re.search(
+        r"from web_interface\.blueprints\.composer import composer_bp", src
+    ), "app.py never imports composer_bp"
+
+
+def test_composer_blueprint_is_registered_under_composer_prefix():
+    src = _source()
+    match = re.search(
+        r"app\.register_blueprint\(\s*composer_bp\s*,\s*url_prefix\s*=\s*"
+        r"['\"](?P<prefix>[^'\"]*)['\"]",
+        src,
+    )
+    assert match, "app.py never calls app.register_blueprint(composer_bp, ...)"
+    # composer.html and composer-app.js hardcode '/composer/api/...' and
+    # '/composer/api/fonts/...' -- any other prefix serves a page whose own
+    # fetches and @font-face all 404.
+    assert match.group("prefix") == "/composer"
+
+
+def test_composer_blueprint_managers_are_wired_before_registration():
+    """Registered with no managers set means every route 503s or crashes.
+
+    composer.py initialises config_manager/plugin_manager/plugins_dir/
+    project_root to None at import time specifically so app.py can set them
+    -- the same pattern api_v3 and pages_v3 use.
+    """
+    src = _source()
+    register_match = re.search(r"app\.register_blueprint\(\s*composer_bp", src)
+    assert register_match, "app.py never registers composer_bp"
+    before_registration = src[: register_match.start()]
+    for attr in ("config_manager", "plugin_manager", "plugins_dir", "project_root"):
+        assert re.search(rf"composer_bp\.{attr}\s*=", before_registration), (
+            f"composer_bp.{attr} is never set before registration"
+        )

test/test_composer_code_injection.py

@@ -67,7 +67,7 @@ def _module_level_code(src):
     tree = ast.parse(src)
     out = []
     for node in tree.body:
-        if isinstance(node, (ast.ClassDef, ast.FunctionDef, ast.ImportFrom)):
+        if isinstance(node, (ast.ClassDef, ast.FunctionDef, ast.ImportFrom, ast.Import)):
             continue
         if isinstance(node, ast.Expr) and isinstance(node.value, ast.Constant):
             continue  # the docstring
@@ -145,6 +145,28 @@ def test_colour_channels_are_clamped_to_a_byte():
     assert "(255, 0, 128)" in src, "channels were not clamped to 0-255"
 
 
+#: fillR/fillG/fillB (and outR/G/B, bgR/G/B, trackR/G/B) go through
+#: _as_fill_filter -> _as_rgb_filter, a separate path from _rgb_expr above.
+#: It used raw int() until it was found to raise ValueError on a non-numeric
+#: channel instead of clamping like every other coerced value in this module.
+@pytest.mark.parametrize("evil", EXPR_PAYLOADS)
+@pytest.mark.parametrize("channel", ["fillR", "fillG", "fillB"])
+def test_a_non_numeric_fill_channel_cannot_reach_the_source(evil, channel):
+    el = {"type": "rectangle", "id": "r1", "x": 0, "y": 0, "width": 10, "height": 8,
+          "fillR": 0, "fillG": 0, "fillB": 128}
+    el[channel] = evil
+    src = _generated(_payload(elements=[el]))
+    assert "__import__" not in src and "os.system" not in src
+    assert not _module_level_code(src)
+
+
+def test_fill_channels_are_clamped_to_a_byte():
+    el = {"type": "rectangle", "id": "r1", "x": 0, "y": 0, "width": 10, "height": 8,
+          "fillR": 99999, "fillG": -5, "fillB": 128}
+    src = _generated(_payload(elements=[el]))
+    assert "(255, 0, 128)" in src, "fill channels were not clamped to 0-255"
+
+
 def test_the_generated_module_still_has_no_top_level_statements():
     """The clean case: a normal payload produces only imports and a class."""
     el = {"type": "text", "id": "t1", "x": 4, "y": 4, "text": "hi",

test/test_composer_empty_block.py

@@ -17,6 +17,7 @@ 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
@@ -78,3 +79,46 @@ def test_template_closes_the_branch_chain_with_a_fallback():
     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

test/test_composer_js_contracts.py

@@ -185,3 +185,79 @@ def test_align_clears_the_anchor_and_moves_line_endpoints():
         "element resolves to the wrong edge"
     assert "el.x0" in body and "el.y0" in body, \
         "_alignElement no longer moves line endpoints"
+
+
+def test_align_translates_both_line_endpoints_not_just_the_start():
+    """Setting only x0 (or y0) left x1/y1 behind, so aligning a line changed
+    its shape instead of moving it -- e.g. a line from x0=20 to x1=50 aligned
+    right became x0=98, x1=50, stretching rather than translating it. Both
+    endpoints must move by the same delta."""
+    body = _method_source(APP, "_alignElement")
+    x_branch = body[body.index("if (axis === 'x')"):body.index("} else {")]
+    y_branch = body[body.index("} else {"):]
+    assert "el.x1" in x_branch, \
+        "_alignElement moves x0 but not x1 -- a line's shape changes, not its position"
+    assert "el.y1" in y_branch, \
+        "_alignElement moves y0 but not y1 -- a line's shape changes, not its position"
+
+
+def test_divider_stroke_is_centered_in_led_pixels():
+    """A 0.5 canvas-pixel offset (correct only at SCALE=1) was applied after
+    scaling instead of before it, so at SCALE>1 the stroke bled into the
+    preceding LED row/column instead of straddling its own."""
+    body = _function_source(CANVAS, "_drawElement")
+    divider_branch = body[body.index("case 'divider'"):]
+    divider_branch = divider_branch[:divider_branch.index("case 'pips'")]
+    assert "ay * s + 0.5" not in divider_branch, \
+        "divider still offsets by 0.5 canvas pixels after scaling"
+    assert "ax * s + 0.5" not in divider_branch, \
+        "divider still offsets by 0.5 canvas pixels after scaling"
+    assert "(ay + 0.5) * s" in divider_branch and "(ax + 0.5) * s" in divider_branch, \
+        "divider stroke is not centered within its LED pixel"
+
+
+def test_gauge_radii_are_clamped_to_zero():
+    """An imported design can carry a small gauge with a wide lineWidth --
+    width=1, height=1, lineWidth=3 sends a negative radius into
+    ctx.ellipse(), which throws IndexSizeError and aborts render() for every
+    element still to be drawn, not just the gauge."""
+    body = _function_source(CANVAS, "_drawElement")
+    gauge_branch = body[body.index("case 'gauge'"):]
+    assert re.search(r"Math\.max\(0,\s*rx\s*-\s*lwPx\s*/\s*2\)", gauge_branch), \
+        "gauge x-radius is not clamped to zero"
+    assert re.search(r"Math\.max\(0,\s*ry\s*-\s*lwPx\s*/\s*2\)", gauge_branch), \
+        "gauge y-radius is not clamped to zero"
+    assert "rx - lwPx / 2" not in re.sub(r"Math\.max\(0,\s*rx\s*-\s*lwPx\s*/\s*2\)", "", gauge_branch), \
+        "an unclamped gauge radius is still passed to ctx.ellipse()"
+
+
+#: ELEMENT_DEFAULTS in composer-canvas.js gives exactly these types a
+#: `binding` object; verified against that file rather than assumed.
+BOUND_TYPES = ["dynamic_text", "progress_bar", "countdown", "pips", "sparkline", "gauge"]
+
+
+def test_bound_types_constant_lists_every_bound_element_type():
+    """ELEMENT_DEFAULTS in composer-canvas.js gives dynamic_text, progress_bar,
+    countdown, pips, sparkline and gauge a `binding` object. If a type is added
+    to (or removed from) that list without updating APP's BOUND_TYPES, the
+    checks below drift out of sync silently -- this pins the two together."""
+    src = APP.read_text()
+    match = re.search(r"const BOUND_TYPES = \[([^\]]*)\];", src)
+    assert match, "BOUND_TYPES constant not found in composer-app.js"
+    declared = {t.strip().strip("'\"") for t in match.group(1).split(",") if t.strip()}
+    assert declared == set(BOUND_TYPES), \
+        f"BOUND_TYPES {declared} does not match the bound element types {set(BOUND_TYPES)}"
+
+
+@pytest.mark.parametrize("method", ["_isBound", "removeConfigVar", "_validateBeforeExport"])
+def test_binding_checks_cover_every_bound_element_type(method):
+    """Only dynamic_text and progress_bar were checked, so a countdown, pips,
+    sparkline or gauge element with an empty binding passed export validation
+    silently, and deleting a config var still used by one of them gave no
+    warning -- the plugin would read a now-missing key at runtime with no
+    indication why."""
+    body = _method_source(APP, method)
+    assert "BOUND_TYPES.includes(e.type)" in body, \
+        f"{method} does not check every bound element type via BOUND_TYPES"
+    assert "e.type === 'dynamic_text'" not in body, \
+        f"{method} still hardcodes only dynamic_text instead of BOUND_TYPES"

web_interface/app.py

@@ -83,11 +83,11 @@ plugin_system_config = config.get('plugin_system', {})
 plugins_dir_name = plugin_system_config.get('plugins_directory', 'plugin-repos')
 
 # Resolve plugin directory - handle both absolute and relative paths
+project_root = Path(__file__).parent.parent
 if os.path.isabs(plugins_dir_name):
     plugins_dir = Path(plugins_dir_name)
 else:
     # If relative, resolve relative to the project root (LEDMatrix directory)
-    project_root = Path(__file__).parent.parent
     plugins_dir = project_root / plugins_dir_name
 
 plugin_manager = PluginManager(
@@ -157,6 +157,7 @@ _health_monitor_initialized = False
 # Register blueprints
 from web_interface.blueprints.pages_v3 import pages_v3
 from web_interface.blueprints.api_v3 import api_v3
+from web_interface.blueprints.composer import composer_bp
 
 # Initialize managers in blueprints
 pages_v3.config_manager = config_manager
@@ -173,6 +174,11 @@ api_v3.operation_queue = operation_queue
 api_v3.plugin_state_manager = plugin_state_manager
 api_v3.operation_history = operation_history
 api_v3.health_monitor = health_monitor
+
+composer_bp.config_manager = config_manager
+composer_bp.plugin_manager = plugin_manager
+composer_bp.plugins_dir = plugins_dir
+composer_bp.project_root = project_root
 # Initialize cache manager for API endpoints
 from src.cache_manager import CacheManager
 api_v3.cache_manager = CacheManager()
@@ -200,6 +206,7 @@ except Exception as _hm_err:  # pragma: no cover - defensive startup guard
 app.register_blueprint(pages_v3, url_prefix='')
 app.register_blueprint(pages_v3, url_prefix='/v3', name='pages_v3_legacy')
 app.register_blueprint(api_v3, url_prefix='/api/v3')
+app.register_blueprint(composer_bp, url_prefix='/composer')
 
 # Route to serve plugin asset files (registered on main app, not blueprint, for /assets/... path)
 @app.route('/assets/plugins/<plugin_id>/uploads/<path:filename>', methods=['GET'])

web_interface/blueprints/composer.py

@@ -95,8 +95,10 @@ def _as_rgb_filter(val) -> str:
     if val is None:
         return 'None'
     # nosemgrep: not a Flask route -- a Jinja filter emitting a Python
-    # tuple literal, with every channel coerced by int().
-    return f'({int(val[0])}, {int(val[1])}, {int(val[2])})'  # nosemgrep
+    # tuple literal, with every channel coerced and clamped by _safe_int().
+    return (f'({_safe_int(val[0], 0, 0, 255)}, '  # nosemgrep
+            f'{_safe_int(val[1], 0, 0, 255)}, '
+            f'{_safe_int(val[2], 0, 0, 255)})')
 
 
 def _as_fill_filter(val) -> str:
@@ -278,7 +280,7 @@ def _preprocess_elements(elements: list) -> list:
 
         x_anchor = el.get('xAnchor') or None
         y_anchor = el.get('yAnchor') or None
-        p['min_width'] = int(el.get('minWidth', 0) or 0)
+        p['min_width'] = _safe_int(el.get('minWidth', 0) or 0, 0, 0, 4096)
 
         if t in ('text', 'clock'):
             font_key = el.get('font', 'press_start')
@@ -290,7 +292,7 @@ def _preprocess_elements(elements: list) -> list:
             p['y_expr'] = _compute_pos_expr(el.get('y', 0), y_anchor, 'height')
             font_size = _FONT_SIZE_MAP.get(font_key, 8)
             char_w = _FONT_CHAR_W.get(font_key, 8)
-            line_spacing = int(el.get('lineSpacing', 2))
+            line_spacing = _safe_int(el.get('lineSpacing', 2), 2, 0, 256)
             y_expr = p['y_expr']
             p['y2_expr'] = f"({y_expr}) + {font_size + line_spacing}"
             if t == 'text':
@@ -377,8 +379,8 @@ def _preprocess_elements(elements: list) -> list:
         elif t == 'progress_bar':
             p['x_expr'] = _compute_pos_expr(el.get('x', 0), x_anchor, 'width')
             p['y_expr'] = _compute_pos_expr(el.get('y', 0), y_anchor, 'height')
-            p['bar_width'] = int(el.get('barWidth', 40))
-            p['bar_height'] = int(el.get('barHeight', 6))
+            p['bar_width'] = _safe_int(el.get('barWidth', 40), 40, 0, 4096)
+            p['bar_height'] = _safe_int(el.get('barHeight', 6), 6, 0, 4096)
             binding = el.get('binding', {})
             p['binding_key'] = binding.get('key', '')
             p['fill_tuple'] = _rgb_tuple(el, ('r', 'g', 'b'), (100, 200, 100))
@@ -403,8 +405,8 @@ def _preprocess_elements(elements: list) -> list:
             p['y_expr'] = y_expr
             p['x2_expr'] = f"({x_expr}) + {w}"
             p['y2_expr'] = f"({y_expr}) + {h}"
-            p['start_angle'] = int(el.get('startAngle', 0))
-            p['end_angle'] = int(el.get('endAngle', 270))
+            p['start_angle'] = _safe_int(el.get('startAngle', 0), 0, -3600, 3600)
+            p['end_angle'] = _safe_int(el.get('endAngle', 270), 270, -3600, 3600)
             p['line_width'] = _safe_int(el.get('lineWidth'), 2, 1, 64)
             p['rgb_tuple'] = _rgb_expr(el, 255, 200, 0)
             p['blink'] = bool(el.get('blink', False))
@@ -445,7 +447,7 @@ def _preprocess_elements(elements: list) -> list:
             p['y_expr'] = y_expr
             p['x2_expr'] = f"({x_expr}) + {w}"
             p['y2_expr'] = f"({y_expr}) + {h}"
-            p['border_radius'] = int(el.get('borderRadius', 3))
+            p['border_radius'] = _safe_int(el.get('borderRadius', 3), 3, 0, 128)
             fill = (
                 [el.get('fillR', 0), el.get('fillG', 80), el.get('fillB', 180)]
                 if el.get('hasFill', True) else None
@@ -473,9 +475,9 @@ def _preprocess_elements(elements: list) -> list:
         elif t == 'pips':
             p['x_expr'] = _compute_pos_expr(el.get('x', 0), x_anchor, 'width')
             p['y_expr'] = _compute_pos_expr(el.get('y', 0), y_anchor, 'height')
-            p['pip_count'] = max(1, int(el.get('count', 5)))
-            p['pip_size'] = max(1, int(el.get('pipSize', 4)))
-            p['pip_spacing'] = max(0, int(el.get('pipSpacing', 2)))
+            p['pip_count'] = _safe_int(el.get('count', 5), 5, 1, 256)
+            p['pip_size'] = _safe_int(el.get('pipSize', 4), 4, 1, 256)
+            p['pip_spacing'] = _safe_int(el.get('pipSpacing', 2), 2, 0, 256)
             p['show_empty'] = bool(el.get('showEmpty', True))
             binding = el.get('binding', {})
             p['binding_key'] = binding.get('key', '')
@@ -488,10 +490,10 @@ def _preprocess_elements(elements: list) -> list:
             y_expr = _compute_pos_expr(el.get('y', 0), y_anchor, 'height')
             p['x_expr'] = x_expr
             p['y_expr'] = y_expr
-            p['bar_width_px'] = int(el.get('width', 40))
-            p['bar_height_px'] = int(el.get('height', 12))
-            p['bar_count'] = max(1, int(el.get('barCount', 8)))
-            p['bar_spacing'] = max(0, int(el.get('barSpacing', 1)))
+            p['bar_width_px'] = _safe_int(el.get('width', 40), 40, 0, 4096)
+            p['bar_height_px'] = _safe_int(el.get('height', 12), 12, 0, 4096)
+            p['bar_count'] = _safe_int(el.get('barCount', 8), 8, 1, 256)
+            p['bar_spacing'] = _safe_int(el.get('barSpacing', 1), 1, 0, 256)
             binding = el.get('binding', {})
             p['binding_key'] = binding.get('key', '')
             p['fill_tuple'] = _rgb_tuple(el, ('r', 'g', 'b'), (80, 200, 120))
@@ -508,8 +510,8 @@ def _preprocess_elements(elements: list) -> list:
             p['y_expr'] = y_expr
             p['x2_expr'] = f"({x_expr}) + {w}"
             p['y2_expr'] = f"({y_expr}) + {h}"
-            p['start_angle'] = int(el.get('startAngle', 135))
-            p['end_angle'] = int(el.get('endAngle', 45))
+            p['start_angle'] = _safe_int(el.get('startAngle', 135), 135, -3600, 3600)
+            p['end_angle'] = _safe_int(el.get('endAngle', 45), 45, -3600, 3600)
             p['line_width'] = _safe_int(el.get('lineWidth'), 3, 1, 64)
             p['rgb_tuple'] = _rgb_expr(el, 80, 220, 80)
             track = (
@@ -532,8 +534,8 @@ def _preprocess_elements(elements: list) -> list:
             p['y_expr'] = _compute_pos_expr(el.get('y', 0), y_anchor, 'height')
             p['text'] = el.get('text', 'Scrolling text')
             p['char_w'] = _FONT_CHAR_W.get(font_key, 8)
-            p['gap'] = int(el.get('gap', 16))
-            p['scroll_speed'] = max(1, int(el.get('scrollSpeed', 1)))
+            p['gap'] = _safe_int(el.get('gap', 16), 16, 0, 4096)
+            p['scroll_speed'] = _safe_int(el.get('scrollSpeed', 1), 1, 1, 256)
             p['direction'] = el.get('direction', 'left')
             # Data key stored in self._data for stateful scrolling across
             # display() calls. It is spliced UNQUOTED into variable names

web_interface/static/v3/js/composer/composer-app.js

@@ -182,6 +182,14 @@ const LED_PALETTE = [
 
 // ── Autosave helpers ─────────────────────────────────────────────────────────
 const LS_KEY = 'ledmatrix_composer_draft';
+
+//: Element types whose ELEMENT_DEFAULTS carry a `binding` object (see
+//: composer-canvas.js). Kept in one place so export validation, the "is this
+//: config var still used" check, and the removal warning cannot drift apart
+//: the way they did when only dynamic_text/progress_bar were checked and
+//: countdown/pips/sparkline/gauge silently went unvalidated.
+const BOUND_TYPES = ['dynamic_text', 'progress_bar', 'countdown', 'pips', 'sparkline', 'gauge'];
+
 let _autosaveTimer = null;
 
 function _debouncedAutosave(payload) {
@@ -572,7 +580,7 @@ function composerApp() {
       link.download = (this.metadata.id || 'composer-design') + '.composer.json';
       link.href = url;
       link.click();
-      URL.revokeObjectURL(url);
+      setTimeout(() => URL.revokeObjectURL(url), 0);
     },
 
     importDesign() {
@@ -583,6 +591,9 @@ function composerApp() {
         const file = e.target.files[0];
         if (!file) return;
         const reader = new FileReader();
+        reader.onerror = () => {
+          this._setStatus('Failed to read file', 'error');
+        };
         reader.onload = (ev) => {
           try {
             const data = JSON.parse(ev.target.result);
@@ -864,15 +875,30 @@ function composerApp() {
                    :                    this.MATRIX_W - bb.w;
         // Clear x-anchor so stored x IS the absolute position
         if ('xAnchor' in el) el.xAnchor = null;
+        if (el.type === 'line') {
+          // Translate both endpoints by the same delta so the line moves
+          // without changing shape -- setting only x0 left x1 behind and
+          // stretched/shrank the line instead of moving it.
+          const dx = Math.round(newX) - bb.x;
+          el.x0 = Math.round(el.x0 + dx);
+          el.x1 = Math.round(el.x1 + dx);
+          el.x = el.x0;
+        } else {
           el.x = Math.round(newX);
-        if (el.type === 'line') el.x0 = Math.round(newX);
+        }
       } else {
         const newY = mode === 'start'  ? 0
                    : mode === 'center' ? Math.round((this.MATRIX_H - bb.h) / 2)
                    :                    this.MATRIX_H - bb.h;
         if ('yAnchor' in el) el.yAnchor = null;
+        if (el.type === 'line') {
+          const dy = Math.round(newY) - bb.y;
+          el.y0 = Math.round(el.y0 + dy);
+          el.y1 = Math.round(el.y1 + dy);
+          el.y = el.y0;
+        } else {
           el.y = Math.round(newY);
-        if (el.type === 'line') el.y0 = Math.round(newY);
+        }
       }
       this._snapshot();
       this.isDirty = true;
@@ -1159,7 +1185,7 @@ function composerApp() {
 
     removeConfigVar(key) {
       const bound = this.elements.filter(
-        e => e.type === 'dynamic_text' && e.binding?.source === 'config' && e.binding?.key === key
+        e => BOUND_TYPES.includes(e.type) && e.binding?.source === 'config' && e.binding?.key === key
       );
       if (bound.length && !confirm(`"${key}" is used by ${bound.length} element(s). Remove anyway?`)) return;
       this.dataModel.configVars = this.dataModel.configVars.filter(v => v.key !== key);
@@ -1169,7 +1195,7 @@ function composerApp() {
 
     _isBound(key) {
       return this.elements.some(
-        e => e.type === 'dynamic_text' && e.binding?.source === 'config' && e.binding?.key === key
+        e => BOUND_TYPES.includes(e.type) && e.binding?.source === 'config' && e.binding?.key === key
       );
     },
 
@@ -1303,7 +1329,7 @@ function composerApp() {
       if (!this.metadata.author.trim()) { this._setStatus('Author is required', 'error'); return false; }
       if (this.elements.length === 0)   { this._setStatus('Add at least one element', 'error'); return false; }
       const unbound = this.elements.filter(
-        e => (e.type === 'dynamic_text' || e.type === 'progress_bar') && !e.binding?.key
+        e => BOUND_TYPES.includes(e.type) && !e.binding?.key
       );
       if (unbound.length) { this._setStatus(`${unbound.length} element(s) have no variable bound`, 'error'); return false; }
       return true;
@@ -1326,7 +1352,7 @@ function composerApp() {
         const url = URL.createObjectURL(blob);
         const a = document.createElement('a');
         a.href = url; a.download = `${this.metadata.id}.zip`; a.click();
-        URL.revokeObjectURL(url);
+        setTimeout(() => URL.revokeObjectURL(url), 0);
         this.generateStatus = 'done';
         this._setStatus('Plugin ZIP downloaded', 'success');
         setTimeout(() => { if (this.generateStatus === 'done') this.generateStatus = 'idle'; }, 4000);

web_interface/static/v3/js/composer/composer-canvas.js

@@ -486,11 +486,11 @@ window.ComposerCanvas = (() => {
           ctx.lineWidth = s;
           ctx.beginPath();
           if (isH) {
-            ctx.moveTo(0, ay * s + 0.5);
-            ctx.lineTo(_canvas.width, ay * s + 0.5);
+            ctx.moveTo(0, (ay + 0.5) * s);
+            ctx.lineTo(_canvas.width, (ay + 0.5) * s);
           } else {
-            ctx.moveTo(ax * s + 0.5, 0);
-            ctx.lineTo(ax * s + 0.5, _canvas.height);
+            ctx.moveTo((ax + 0.5) * s, 0);
+            ctx.lineTo((ax + 0.5) * s, _canvas.height);
           }
           ctx.stroke();
           break;
@@ -560,13 +560,17 @@ window.ComposerCanvas = (() => {
             ? Math.max(0, Math.min(100, parseFloat(pvGauge) || 0)) / 100
             : Math.max(0, Math.min(100, el.previewPct ?? 65)) / 100;
           const fillSweep = totalSweep * pct;
+          // A small imported gauge with a wide lineWidth would otherwise send
+          // a negative radius into ctx.ellipse(), which throws IndexSizeError
+          // and aborts the whole render() call.
+          const arx = Math.max(0, rx - lwPx / 2), ary = Math.max(0, ry - lwPx / 2);
           // biome-ignore lint/correctness/useQwikValidLexicalScope: not Qwik -- this is an Alpine.js component, and the rule is about Qwik's $() serialization boundary, which does not exist here.
           const toRad = deg => (deg - 90) * Math.PI / 180; // canvas 0=top, PIL 0=right → offset -90
 
           // Track arc
           if (el.hasTrack !== false) {
             ctx.beginPath();
-            ctx.ellipse(cx, cy, rx - lwPx / 2, ry - lwPx / 2, 0, toRad(startDeg), toRad(startDeg + totalSweep), false);
+            ctx.ellipse(cx, cy, arx, ary, 0, toRad(startDeg), toRad(startDeg + totalSweep), false);
             ctx.strokeStyle = `rgb(${el.trackR ?? 40},${el.trackG ?? 40},${el.trackB ?? 40})`;
             ctx.lineWidth = lwPx;
             ctx.stroke();
@@ -574,7 +578,7 @@ window.ComposerCanvas = (() => {
           // Fill arc
           if (pct > 0) {
             ctx.beginPath();
-            ctx.ellipse(cx, cy, rx - lwPx / 2, ry - lwPx / 2, 0, toRad(startDeg), toRad(startDeg + fillSweep), false);
+            ctx.ellipse(cx, cy, arx, ary, 0, toRad(startDeg), toRad(startDeg + fillSweep), false);
             ctx.strokeStyle = `rgb(${el.r},${el.g},${el.b})`;
             ctx.lineWidth = lwPx;
             ctx.stroke();

web_interface/templates/v3/composer/manager.py.j2

@@ -110,6 +110,8 @@ class {{ class_name }}(BasePlugin):
 {{ pi }}    color={{ el.rgb_tuple }},
 {{ pi }}    font=self.display_manager.{{ el.font_attr }},
 {{ pi }})
+{% else %}
+{{ pi }}pass  # non-config dynamic_text binding draws nothing
 {% endif %}
 {% elif el.type == 'clock' %}
 {{ pi }}self.display_manager.draw_text(