mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-23 03:18:15 +00:00
2f42d179f68f2b4782b72bc8393a69fd0734a97d
17
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 ChuckBuildsandClaude Opus 5
|
2f42d179f6 |
fix(composer): point the align toolbar at the anchor-clearing path
Two alignment implementations existed and the toolbar used the wrong one. alignElement(dir) set el.x/el.y and stopped there. resolveAnchor turns anchor='right' into `dim - val`, so with xAnchor='right' an "align left" (el.x = 0) resolved to x = MATRIX_W and the element jumped to the far right edge -- the opposite of what was asked. It also never touched el.x0/el.y0, so a line's endpoints were left where they were. _alignElement already did both correctly: it clears the anchor so the stored value is absolute, and moves x0/y0 for lines. Its six wrappers -- alignLeft, alignHCenter, alignRight, alignTop, alignVCenter, alignBottom -- existed and had no callers at all. All six toolbar buttons now call the wrappers, and the legacy method is removed rather than left to drift back into use. Tests: the toolbar calls each wrapper and no longer calls alignElement, the legacy definition is gone, and _alignElement still clears the anchor and moves line endpoints. Two of them fail against the previous markup. Full suite 4062 passed, the one failure being test_install_lowmem (pre-existing, awaiting #492). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW |
||
|
ChuckBuildsandClaude Opus 5
|
37fc1b56b5 |
fix(composer): coerce prefixed colour channels, non-finite numbers, marquee ids
Three more routes into the generated source, plus a fix to one of my own tests
that was checking the wrong branch.
Prefixed colour channels were interpolated raw
----------------------------------------------
Five tuples were built without coercion:
p['fill_tuple'] = f"({el.get('r', 100)}, {el.get('g', 200)}, ...)"
p['empty_tuple'] = f"({el.get('emptyR', 50)}, ...)"
p['label_tuple'] = f"({el.get('labelR', 200)}, ...)"
so progress_bar, pips, sparkline and gauge took arbitrary expressions the same
way width/height did. Confirmed: every one of the five put __import__ into the
generated source. They now go through a new _rgb_tuple helper, which _rgb_expr
also delegates to.
The pre-existing colour test only covered r/g/b on a text element, which is why
the prefixed channels and these four types were never exercised.
Non-finite numbers escaped as a 500
-----------------------------------
json.loads accepts Infinity/-Infinity/NaN by default and Flask's get_json
passes them straight through, so a payload can hand _safe_int a non-finite
float. int(inf) raises OverflowError, which is neither ValueError nor
ComposerInputError, so it escaped both handlers and surfaced as a 500 with a
traceback rather than a 422. Verified end to end through Flask's parser.
Marquee ids reached the source as identifiers
---------------------------------------------
data_key is spliced UNQUOTED into variable names (_{{ data_key }}_text = ...)
and only '-' was normalised. A punctuated id landed in the generated source as
code. ast.parse caught it, so this was not exploitable, but the caller got an
opaque "Generated code has a syntax error" instead of being told the id was
unusable -- the same failure mode as the empty-block bug. Now restricted to
identifier characters and bounded to 64.
The line-anchor test was testing the wrong branch
-------------------------------------------------
test_line_branch_applies_the_anchor_offset searched the whole file for
"case 'line': {". getBoundingBox has one too and comes first, so the assertion
was reading the bounding-box branch: stripping the anchor offset from
_drawElement left all 11 checks green. Both line tests are now scoped to their
own function via tree-sitter, so they cannot be satisfied by the same branch.
Tests: 35 of the injection suite's checks fail against the reverted fixes; the
scoped line test fails when _drawElement's offset is removed. Full suite 4059
passed, the one failure being test_install_lowmem (pre-existing, awaiting #492).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
acc55ef119 |
fix(composer): scale strokes, anchor lines, snapshot state changes
Three review findings in the composer's JavaScript, all confirmed against the
code.
Stroke widths did not scale with SCALE
--------------------------------------
_drawElement scales all geometry by `s`, but left ctx.lineWidth in canvas
pixels, so at SCALE>1 every outline rendered thinner than one LED pixel and
the preview stopped matching the panel it is previewing. Fixed for rectangle,
ellipse, arc, rounded_rectangle, line, divider and progress_bar. Ellipse and
arc also inset their radii by half the scaled width -- a stroke straddles its
path, so without the inset the outline spills outside the element's bounds.
The gauge branch already did this; the rest now match it.
Selection handles and the grid stay in canvas pixels deliberately: they are
editor chrome, not LED geometry, and live in other functions.
`line` ignored anchors
----------------------
_drawElement resolves ax/ay for every element, but the line branch drew raw
el.x0/el.y0/el.x1/el.y1. Setting xAnchor or yAnchor moved every other element
type and left lines where they were. getBoundingBox had the same omission, so
even once a line moved its hit box would not have. Both now translate by
(ax - el.x0, ay - el.y0); ax resolves from el.x0 for a line, so that is
exactly the anchor offset.
Four state mutations skipped _snapshot
--------------------------------------
_snapshot serialises metadata and currentPreset and is the only caller of
_debouncedAutosave. onBgColorChange, setCustomSize, changePreset and
applyPresetLabel each changed exactly those values without calling it, so the
background colour and the canvas size were lost on reload and could not be
undone. Same defect already fixed in onColorChange.
The review named three; applyPresetLabel has it too -- it is the branch that
handles sizes absent from DISPLAY_PRESETS.
Snapshotting is on the user-driven path only. _applyState and loadTemplate
drive these with {silent: true} while restoring, and snapshotting there would
push restore steps onto the undo stack and re-autosave the state just loaded.
Tests
-----
No JS runner here, so test_composer_js_contracts.py asserts on the parse tree
via tree-sitter: both files parse, no bare `ctx.lineWidth = 1` inside
_drawElement, the line branch and its bounding box carry the anchor offset,
each of the five mutations snapshots, and the two preset paths keep their
!opts.silent guard ahead of the snapshot.
9 of its 11 checks fail against the previous JS. Full suite 3978 passed, the
one failure being test_install_lowmem (pre-existing, awaiting #492).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
1a0864e5d4 |
fix(composer): clamp width/height before they reach generated source
Code injection, found by chasing why a security test could not have caught it.
_preprocess_elements built the far corner of five shapes by interpolating the
payload's width/height straight into generated Python:
w = el.get('width', 10)
p['x2_expr'] = f"({x_expr}) + {w}"
so a rectangle with width='0 or __import__("os").system("id")' generated
[0, 0, (0) + 0 or __import__("os").system("id"), (0) + 8],
inside a manager.py that /api/install writes to disk and the plugin loader
imports and executes. rectangle, arc, ellipse, rounded_rectangle and gauge all
share the pattern. Both fields now go through _safe_int, like every other
geometry value.
Unreachable today only because composer_bp is still unregistered -- the same
caveat as the docstring injection fixed earlier in this PR.
Why the existing test missed it
-------------------------------
test_a_non_numeric_geometry_value_cannot_reach_the_source drove its payloads
through a "line" element. manager.py.j2 has never had a `line` branch, so
_preprocess_elements produced nothing for it and no value it set could reach
the generated source. Every assertion passed trivially, against code that was
in fact vulnerable. The test has been vacuous since it was written; the
_RENDERABLE_ELEMENT_TYPES constant added in the previous commit only made the
cause legible.
It now runs across the five types that actually render, over x/y/width/height:
40 of those cases fail with the clamping reverted, where the old version
passed 100%.
A second test asserts every type used by the injection suite is in
_RENDERABLE_ELEMENT_TYPES, so the suite cannot quietly go vacuous again.
Also: _payload set "config_vars", but _generate_plugin_files reads
data['dataModel']['configVars']. Nothing passed through that key was ever
read. Fixed so config-var tests exercise the real path.
Full suite: 3967 passed, the one failure being test_install_lowmem
(pre-existing, awaiting #492).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
732c7d1a30 |
fix(composer): an element the template cannot draw broke generation
Reproduced from the review comment. A `group` element carrying minWidth
generated:
if width >= 64: # breakpoint: 64px+ displays only
# <nothing>
manager.py.j2 wraps each element in the breakpoint and blink blocks, but the
body comes from the per-type branches -- and a type with no branch contributes
nothing, so the wrapper opens a block with no statements. ast.parse then fails
and the caller is told only "Generated code has a syntax error: expected an
indented block ... line 49", naming a line of generated source they never see.
Two defences:
- _preprocess_elements drops types the template has no branch for, alongside
the existing `section` skip. This is the root cause: those elements should
never have reached the template.
- The branch chain ends in `{% else %}pass`, so a type added to the canvas
before its drawing branch exists degrades to a no-op rather than a plugin
that will not parse.
The review also cited dynamic_text with binding_source != 'config'. That one
does not reproduce -- the branch emits a draw_text regardless -- which is why
an earlier attempt to reproduce this found nothing.
_RENDERABLE_ELEMENT_TYPES has to stay in step with the template: a type listed
with no branch emits an empty block again, and a branch missing from the list
is silently dropped from every generated plugin. A test asserts the two sets
are equal rather than trusting them to be maintained together.
Tests: 12 new, covering group/unknown/section against breakpoint, blink and
both nested, plus the set-equality and fallback checks. 7 fail with both
defences reverted. 172 composer tests pass; full suite 3862 passed, the one
failure being test_install_lowmem (pre-existing, awaiting #492).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
d42593e7ce |
Merge main into feat/plugin-composer, and fix three review findings
The branch was 57 commits behind and conflicting. I had put the rebase
aside earlier as needing the author's eyes, on the grounds that the PR is
+5091 lines -- but that was the wrong measure. The actual conflict was a
single hunk in app.css, where this branch adds .md\:inline and main added
.md\:block and .md\:w-auto at the same place. All three are kept.
Merging rather than rebasing: the branch is public and 57 commits behind,
so a rebase would rewrite shared history for a force-push.
Three findings fixed on top:
A missing `text` or `format` was a 500. `p` is a copy of the raw element
and the defaults were applied to the locals t1/fmt1 only, so an element
omitting either key left it absent, manager.py.j2 rendered
`{{ el.text | tojson }}` over a jinja2.Undefined, and tojson raised
TypeError -- which no handler catches:
text without 'text': TypeError: Object of type Undefined is not
JSON serializable
clock without 'format': same
Both keys are now set explicitly. Verified: removing either assignment
fails 4 of the new tests.
E741 on my own injection-test file: two `for i, l in enumerate(...)`
loops, which ruff rejects and would fail a lint-gated build. Renamed.
Ruff now clean on all three files this PR touches.
Not done: registering composer_bp. This PR's own description gates it --
"Not yet wired up ... tracking as a follow-up", with an unchecked box for
"Register composer_bp in app.py before merging or exposing this route" --
so it is a deliberate decision, not an oversight. Confirmed the blueprint
appears in no register_blueprint call outside this branch's tests, which
also means the code-injection fixed earlier in this PR was never
reachable in a deployed instance. Worth fixing before the route is
exposed; not worth exposing the route to satisfy a review comment.
Verified on the merged tree: 3850 passed, 1 failed, 60 skipped. The
failure is test_install_lowmem's tmpfs assumption, which is fixed in #492
and not yet on main. The static audit now passes 3/3 -- the twelve
classes it flagged before were defined on main all along and only looked
missing because this branch was behind.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
f0bef7784c |
fix(composer): eight editor bugs from review
All confirmed by reading the code rather than taken on trust.
Saved designs restored onto the wrong canvas. _buildPayload writes the
size as `preset`; _applyState read `state.currentPreset`, which is never
present, so changePreset(undefined) hit its `if (!preset) return` and did
nothing -- silently. A 256x64 design reopened at 128x32 with every
element misplaced. importDesign passed no size key at all, same result.
Both go through a new applyPresetLabel(), which also handles the custom
labels setCustomSize() writes ("200x50"): those are deliberately absent
from DISPLAY_PRESETS, so changePreset alone could never round-trip them.
Keyboard shortcuts hijacked text fields. The `inInput` guard sat below
the Ctrl/Cmd block, under a comment claiming combos "work everywhere".
In any input, Ctrl+C copied the selected *element* -- preventDefault
stopping the real copy -- Ctrl+V pasted an element, Ctrl+A could not
select the field contents, and Tab always moved the element selection,
so keyboard users could not reach the next input. Guard moved above both
blocks, and it now covers contenteditable too.
Resize handles were advertised on five shapes that ignored them. The
canvas drew handles for six element types; the editor gated resize and
hover on `type === 'rectangle'`. The list was also duplicated inside the
canvas. One exported RESIZABLE_TYPES now feeds all four sites.
Lines jumped on drag. addElement assigns x/y *before* spreading
ELEMENT_DEFAULTS, and the line defaults define only x0/y0 -- so a line
carries both, with x at canvas/4 and x0 at 0. Drag and nudge move x0/y0
only, so _getStoredPos preferring `x` handed the drag a base it never
updates.
Colour-picker edits were lost on reload. onColorChange mutated the
element but never set isDirty or called _snapshot, and _debouncedAutosave
only runs from _snapshot. applyPaletteColor did both; they match now.
Also: section elements drew nothing and reported a 0x0 box, so adding
"Section Label" from the palette looked broken and the element was
selectable only through the 3px hit-test padding -- they now draw their
label, with the bounding box using the same font fallback as the draw
call so the two agree. The gauge inset its arc radius by lw/2 where lw is
LED pixels and the radius is canvas pixels, then stroked at lw*s, so the
arc spilled outside its own bounding box at any scale above 1. And the
plugin id is encodeURIComponent'd before it becomes part of a request
path.
Verified: composer-app.js and composer-canvas.js parse cleanly under
tree-sitter (esprima cannot read this codebase -- it predates ??, and
fails identically on the unmodified files). Every symbol referenced
across module boundaries checked to exist. 156 Python composer tests
pass. The static-audit failure is the same 13 classes as before, all
defined on main and absent only because this branch is behind; nothing
here touches CSS or templates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
986f74e38b |
fix(composer): reject config keys that shadow plugin state
Five review findings, plus the two bandit reported.
Config variable keys were checked against an identifier regex only.
Python keywords slipped past it and were caught downstream by ast.parse,
but reported as
Generated code has a syntax error: invalid syntax (<unknown>, line 17)
which names neither the field nor the value. They are now refused by
name, soft keywords ('match', 'case') included.
Worse, a key matching a BasePlugin attribute generated *valid* code that
silently clobbered plugin state. 'config' is the sharp one: the
assignment lands immediately after super().__init__(), so
self.config = config.get("config", "x")
replaces the plugin's config dict with a string, and every later
self.config.get(...) fails at runtime. Refused now, along with logger,
display_manager, cache_manager, plugin_id, enabled, self and the
lifecycle method names. A test pins the ordering assumption that reserved
list rests on, so it fails if config vars are ever emitted before
super().__init__() instead.
Also:
- The silent `except Exception: pass` around manifest parsing now logs.
It left "partial import produced nothing" indistinguishable from a
malformed manifest. (bandit B110)
- list_plugins() called iterdir() on a directory that may not exist --
a fresh install or a bad path returned 500 instead of an empty list.
- metadata.id is stripped in the two route handlers, matching
_generate_plugin_files, which strips before validating. Without it
" my-plugin " generated fine and then failed the id check at install,
reading as a generator bug.
- The jinja Environment's autoescape=False now says why: these templates
emit Python, and escaping a quote to " inside generated code would
break it. Safety comes from the values instead -- _safe_int, _rgb_expr
and _reject_source_breaking, all covered by the injection suite.
(bandit B701, marked nosec with that rationale)
bandit on composer.py: 2 findings -> 0.
Verified: 156 tests across the two composer suites. Removing either new
key check fails 9.
Not reproduced: the suggestion to emit `pass` so a conditional block is
never empty. 'line' and 'divider' render through a different template
branch and 'section' emits nothing at all, so no element type available
here produces an `if width >= N:` with an empty body. Left alone rather
than changing template output speculatively.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
e450a6dfb6 |
fix(composer): stop payload text reaching generated Python as code
Review flagged this as critical and it is: the composer builds manager.py
by interpolating payload values into source text, /api/install writes
that file into plugins_dir, and the plugin loader imports and executes
it. The ast.parse check further down rejects only *invalid* syntax, and
an injected `import os` is perfectly valid.
Confirmed against the code before this commit. A plugin name carrying a
triple quote closes the module docstring and everything after it becomes
module-level code:
generated manager.py parses: True
injected module-level statements: ['import os', 'PWNED = os.getuid()']
and a geometry value is interpolated verbatim, because the parameter is
annotated int but arrives as JSON:
_compute_pos_expr('0 or __import__("os").system("id")', 'right', 'width')
-> 'width - 0 or __import__("os").system("id")'
generated source: x=0 or __import__("os").system("id"),
Three fixes. _safe_int coerces and optionally clamps, and
_compute_pos_expr applies it to its own argument -- which covers all
twenty-odd call sites at once rather than patching each. _rgb_expr does
the same for the eight colour interpolations, clamping channels to
0-255. Line endpoints and widths go through it too.
For the docstring, _reject_source_breaking refuses a plugin name
containing a quote, backslash or newline. Rejecting rather than escaping:
these are display names, none of that belongs in one, and a clear "Plugin
name cannot contain a double quote." beats silently mangling what the
user typed.
Verified: all three exploits now refused or neutered, and each defence
mutation-checked separately --
coercion removed in _compute_pos_expr -> 8 failed
docstring guard removed -> 5 failed
colour channels interpolated raw -> 13 failed
87 tests, covering seven expression payloads across seven geometry
fields and three colour channels, five literal-breaking names, and the
clean case asserting a normal payload still yields no module-level
statements at all.
One aside: the first version of this test file put the exploit string
in its own module docstring, which closed it and made the file a syntax
error -- the same bug, one level up. It now describes the payload rather
than embedding it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
5133643600 |
fix(composer): build the font path from the allowlist entry
secure_filename cleared the plugin-directory alerts: CodeQL went from 19
to 5, and from 16 high-severity to 2. The two that remain are in
serve_font, which is gated by a frozenset of three exact filenames -- so
nothing was exploitable -- but the name reaching the filesystem was still
the request value.
It now comes from the matched allowlist entry. Identical strings, so
runtime behaviour is unchanged; the difference is that the filename is
provably a module constant rather than a guarded piece of user input.
The first test I wrote for this proved nothing. It asserted 404 on
traversal payloads, but Flask's router will not match a path segment
containing '/', and the rest 404 simply because no such file exists --
so removing the allowlist entirely still passed. Replaced with a readable
file planted next to the fonts:
fonts/id_rsa.ttf -> 404, body does not contain its contents
which fails with "a readable non-allowlisted file was served" the moment
the gate is removed.
45 tests.
Left alone: three medium py/stack-trace-exposure alerts on the
_generate_plugin_files handlers, which return str(exc) for
ComposerInputError. Its seven raise sites are all authored literals
("Author is required.", "Config variable key X is not a valid Python
identifier."), so no traceback or path is exposed. Clearing them means
either replacing that feedback with a generic string or restructuring
validation to return errors instead of raising -- a change to the
author's design, made blind, since CodeQL cannot be run locally to
confirm it would even work. That is a decision, not a cleanup.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
5929190e36 |
fix(composer): sanitise the id with the form CodeQL recognises
Previous attempt got the count from 22 down to 19 but left the 16
path-injection alerts untouched: CodeQL carries taint through
_plugin_dir's return value and does not treat an internal realpath /
commonpath guard as a sanitiser.
secure_filename is one it does model. It is also a no-op on every id the
regex accepts -- verified across the accepted alphabet, 4000 generated
ids, zero altered -- so it cannot rewrite a caller's id into a different
plugin's directory. The equality check makes that explicit: if it changes
anything, the id was not one we accept, and we refuse rather than
silently redirect.
Found a real bug while testing the layers separately: '.' resolved to the
plugins root, and install() calls shutil.rmtree(target) when force is
set, so an id of '.' would have deleted every installed plugin. The regex
blocks it today, but the containment layer was allowing candidate == base
on the grounds that the base is not "outside" itself. A plugin directory
must be a child, never the root.
That came out of writing the isolated tests. Removing containment did not
fail anything, because secure_filename rejects traversal first -- which
made a redundant layer look load-bearing. Each layer is now neutralised
in turn so the one under test is the only thing standing:
containment removed -> FAIL (13 payloads reach the base or past it)
candidate == base allowed -> FAIL ('.' resolves to the plugins root)
commonpath -> startswith -> FAIL (sibling "plugins-evil" accepted)
secure_filename bypassed -> pass, containment covers it
The last is honest rather than a gap: with containment in place the
sanitiser has nothing left to block, and its value here is CodeQL
recognition plus a second barrier if containment is ever weakened.
Also corrected an assertion in the previous commit's test, which counted
any non-None result as an escape. '....', '~' and 'a\..\..' are ordinary
directory names on Linux and resolve safely inside the base; treating
them as escapes made the test fail on correct code.
35 tests. The 5 test_web_api.py failures are pre-existing on this branch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
79ba93f5a6 |
fix(composer): recognisable path containment, and define md:inline
Follow-up to the previous commit, which made the CodeQL count worse
rather than better: 19 alerts became 22. Two mistakes.
First, the containment check used `base not in candidate.parents`.
That is correct Python but not a form static analysis recognises, so
every path-injection alert stayed and _plugin_dir itself picked up two
more. It now uses os.path.realpath plus os.path.commonpath, which is
both the documented sanitiser shape and stricter than the obvious
alternative: "/x/plugins-evil" startswith "/x/plugins" but is a
different directory, and there is now a test that fails if anyone
swaps commonpath for startswith.
Second, raising ComposerInputError from _plugin_dir and returning
str(exc) added two new py/stack-trace-exposure alerts -- CodeQL flags
exception text reaching a response regardless of the exception's type.
_plugin_dir returns None instead and the three handlers answer with a
fixed literal. There is nothing a caller needs there beyond "that id is
not ok".
Also defines .md\:inline in app.css. composer.html marks five toolbar
button labels `hidden md:inline`, and the class was never defined, so
those labels were hidden at every width and the buttons stayed
icon-only. main's test_web_static_audit.py catches it -- the branch
predates that test, which is why it only surfaced now that CI checks
the merge:
Responsive utility classes referenced in templates but never
defined in app.css (they silently no-op): ['md:inline']
Verified against the merged state -- main's app.css plus this one line,
audited against this branch's templates: 3 passed. The other twelve
classes the audit flags locally are defined on main and are artifacts of
this branch being 54 commits behind.
33 containment tests. Mutation-checked twice: removing the containment
lets eight payloads escape, including /etc/passwd and
plugin/../../../../../../etc/shadow; swapping commonpath for startswith
fails the sibling-prefix test.
Not addressed: three py/stack-trace-exposure alerts on the
_generate_plugin_files handlers. Those return str(exc) for
ComposerInputError, whose seven raise sites are all authored literals
("Author is required.", "Config variable key X is not a valid Python
identifier."). Suppressing them means replacing useful validation
feedback with a generic string, which is a real cost to the user for a
scanner's benefit. Worth a decision rather than a silent downgrade.
The 5 test_web_api.py failures are pre-existing on this branch --
identical counts with these changes stashed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
e499efb1f0 |
fix(composer): resolve plugin paths at the filesystem boundary
CodeQL reports 19 alerts against this PR -- 16 high-severity
py/path-injection plus 3 py/stack-trace-exposure -- all in
web_interface/blueprints/composer.py, where a request-supplied plugin_id
reaches Path(plugins_dir) / plugin_id and the result is created, written
to, deleted with shutil.rmtree, and read back.
The path-injection alerts are false positives today. _PLUGIN_ID_RE is
fully anchored and permits only [a-z][a-z0-9-]{0,62}, so every traversal
payload is already rejected; I checked fourteen of them, including
../../etc/passwd, a/../../etc, /etc/passwd and encoded variants, and none
gets past it.
They are worth fixing anyway. The guarantee lived in a regex several
hundred lines from the path building, so relaxing that pattern later --
to allow an underscore, say -- would open a traversal with nothing at the
filesystem boundary to catch it. _plugin_dir() now resolves the candidate
and refuses anything that is not inside plugins_dir, and all three call
sites go through it. That is also the shape static analysis recognises,
which is why sixteen alerts landed on code that was already safe.
The regex anchor moves from $ to \Z. Python's $ also matches just before
a trailing newline, so "myplugin\n" was accepted and would have created a
directory whose name ends in one. Not traversal, but not a name anything
downstream should have to handle.
For the stack-trace exposure: the handlers returned str(exc) for any
ValueError out of _generate_plugin_files. The seven raises there are all
curated, user-facing validation messages, and they now use a
ComposerInputError subclass so they keep reaching the user verbatim. A
ValueError from anywhere else -- json, int(), a library -- is logged with
a traceback and answered generically, since its text can name internal
paths.
Verified: 32 tests covering fourteen traversal payloads and twelve
malformed ids. The key one re-runs every payload with the id pattern
deliberately loosened to allow slashes and dots; removing the containment
check fails it with
these escaped the base with a loosened regex:
[('/etc/passwd', '/etc/passwd'), ('//etc/passwd', '//etc/passwd')]
so the boundary is doing real work rather than shadowing the regex.
The 5 failures in test_web_api.py are unrelated and pre-existing on this
branch -- identical counts with these changes stashed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
||
|
ChuckBuildsandClaude Opus 5
|
183e23edb3 |
docs(changelog): record the compatibility gate in 3.2.0
The 3.2.0 section described the unified sports library but none of the install-path work that landed in #428 and #431 -- which matters more than a normal changelog omission, because the sunset rule keys on this section to tell plugin authors what a given floor buys them. The headline addition: 3.2.0 is the first release that *enforces* ledmatrix_min_version. Before it the floor was advisory, so a plugin could declare one and still be delivered to a core that could not run it. That is the property B6 waits on, and it is now stated where a plugin author will look for it -- along with the caveat that a core reporting below 2.0.0 is treated as unknown rather than old and is never blocked. Also records compatibility.py (and that it does not yet read compatible_versions), check_release_version.py and its workflow, the install-preservation fix, the reentrant-lock deadlock fix, and the web_interface version re-export. No version bump: 3.2.0 is unreleased, so this describes the release being cut rather than a new one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 |
||
|
ChuckBuildsandClaude Sonnet 5
|
cd7e16e58e |
fix(security): validate plugin_id before path construction in /api/install
CodeQL flagged 16 high-severity "path depends on user-provided value" alerts. Investigated each: - install_locally() (/api/install) built a filesystem path from metadata.id without validating it at that point -- it was only implicitly safe because _generate_plugin_files() validates the same field (re-extracted independently) earlier in the same request. That's a real gap: reorder or change that earlier call and it's an exploitable path traversal / arbitrary file write. Fixed by validating plugin_id directly against _PLUGIN_ID_RE at the point the path is built, matching the pattern already used correctly in validate_id() and load_plugin(). - The other 10 flagged locations (serve_font's allowlist check, validate_id, load_plugin and its downstream reads) were already guarded by an explicit check earlier in the same function -- false positives from CodeQL not modeling those as sanitizers. Also fixed 2 of the 5 "stack trace exposed" warnings that were genuine: install_locally() and load_plugin() returned raw OSError/Exception text to the client in a 500 response; now logged server-side with a generic client-facing message. The other 3 (generate_zip/install_locally/ preview_code returning str(ValueError) from _generate_plugin_files) are deliberate, human-authored validation messages, not exception internals -- left as-is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ |
||
|
ChuckBuildsandClaude Sonnet 5
|
47e3021fc3 |
fix: address Codacy findings in the Composer blueprint
- Dropped a pointless f-string prefix (no placeholders) on the default plugin description. - Replaced two bare except:pass/continue blocks (manifest.json listing, config_schema.json parsing) with a logged warning before falling through to the same skip-this-entry behavior -- same control flow, now visible in logs instead of silent. Skipped as false positives (verified against actual usage, not fixed): - Jinja2 Environment(autoescape=False) -- this env renders manager.py.j2, a Python source-code generator, never HTML; autoescaping would corrupt generated code. Flagged by a generic XSS rule that assumes all Jinja2 environments render HTML. - "Flask route directly returning a formatted string" on _as_rgb_filter -- that's a Jinja *filter* function, not a Flask route. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ |
||
|
ChuckBuildsandClaude Sonnet 5
|
e319540c6e |
feat(web): add Plugin Composer -- visual drag-and-drop plugin builder
Web UI (/composer/) for building a working LEDMatrix plugin without writing Python: drop elements (text, time, date, countdown, scrolling text, bar/waveform, groups, custom config variables) onto a canvas matching the real panel's pixel grid, configure them with live preview, then generate a real plugin (manager.py + manifest.json + config_schema.json) from manager.py.j2 -- downloadable as a ZIP or installed directly. NOTE: composer_bp is not yet registered in web_interface/app.py, so this blueprint is currently inert. Split out of the original chore/dead-code- removal commit, which had accidentally bundled this in alongside unrelated dead-code deletions; app.py registration was not part of that commit either and still needs to be added before this is reachable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ |