diff --git a/server/src/ledgrab/api/routes/automations.py b/server/src/ledgrab/api/routes/automations.py index e8f3142..feebbbb 100644 --- a/server/src/ledgrab/api/routes/automations.py +++ b/server/src/ledgrab/api/routes/automations.py @@ -134,13 +134,18 @@ def _action_from_schema(s: ActionSchema) -> Action: fire_on = s.fire_on or "activate" if fire_on not in ("activate", "deactivate", "both"): raise ValueError(f"Invalid fire_on: {fire_on}. Must be activate, deactivate or both.") + # content_type is emitted verbatim as the outbound Content-Type header — reject + # control chars (CR/LF) so it can't be used to inject additional HTTP headers. + content_type = (s.content_type or "application/json").strip() + if len(content_type) > 128 or any(ord(c) < 0x20 or ord(c) > 0x7E for c in content_type): + raise ValueError("Invalid content_type: control or non-ASCII characters are not allowed.") # Raises HTTPException(400) on a blocked/loopback/metadata target. validate_polling_url(url) return WebhookAction( webhook_url=url, method=method, body_template=s.body_template or "", - content_type=s.content_type or "application/json", + content_type=content_type, fire_on=fire_on, ) @@ -487,5 +492,9 @@ async def trigger_automation( except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) - status, errors = await engine.fire_manual_trigger(automation) + try: + status, errors = await engine.fire_manual_trigger(automation) + except Exception as e: # noqa: BLE001 — surface a structured error, never a bare 500 + logger.error("Manual trigger failed for automation %s: %s", automation_id, e) + return AutomationTriggerResponse(status="error", errors=[str(e)]) return AutomationTriggerResponse(status=status, errors=errors) diff --git a/server/src/ledgrab/api/schemas/automations.py b/server/src/ledgrab/api/schemas/automations.py index 4853cc4..c1ffc9e 100644 --- a/server/src/ledgrab/api/schemas/automations.py +++ b/server/src/ledgrab/api/schemas/automations.py @@ -111,12 +111,16 @@ ConditionSchema = RuleSchema class ActionSchema(BaseModel): """A single outbound action fired alongside scene activation/deactivation.""" - action_type: str = Field(description="Action type discriminator (e.g. 'webhook')") + action_type: str = Field( + max_length=32, description="Action type discriminator (e.g. 'webhook')" + ) # Webhook action fields webhook_url: str | None = Field( None, max_length=2048, description="Target URL for the webhook action" ) - method: str | None = Field(None, description="'POST', 'PUT', or 'GET' (for webhook action)") + method: str | None = Field( + None, max_length=8, description="'POST', 'PUT', or 'GET' (for webhook action)" + ) body_template: str | None = Field( None, max_length=8192, @@ -126,10 +130,12 @@ class ActionSchema(BaseModel): ), ) content_type: str | None = Field( - None, description="Content-Type header for the webhook body (default application/json)" + None, + max_length=128, + description="Content-Type header for the webhook body (default application/json)", ) fire_on: str | None = Field( - None, description="'activate', 'deactivate', or 'both' (for webhook action)" + None, max_length=16, description="'activate', 'deactivate', or 'both' (for webhook action)" ) diff --git a/server/src/ledgrab/api/schemas/color_strip_sources.py b/server/src/ledgrab/api/schemas/color_strip_sources.py index 6654f84..881e377 100644 --- a/server/src/ledgrab/api/schemas/color_strip_sources.py +++ b/server/src/ledgrab/api/schemas/color_strip_sources.py @@ -338,7 +338,9 @@ class EffectCSSCreate(_CSSCreateBase): custom_palette: List[List[float]] | None = Field(None, description="Custom palette stops") audio_reactive: bool | None = Field(None, description="Modulate output by live audio loudness") reactive_audio_source_id: str | None = Field(None, description="AudioSource id for reactivity") - reactive_mode: str | None = Field(None, description="brightness | saturation | both") + reactive_mode: Literal["brightness", "saturation", "both"] | None = Field( + None, description="brightness | saturation | both" + ) reactive_intensity: Any = Field(default=None, description="Reactive modulation strength (0-1)") @@ -542,7 +544,9 @@ class EffectCSSUpdate(_CSSUpdateBase): custom_palette: List[List[float]] | None = Field(None, description="Custom palette stops") audio_reactive: bool | None = Field(None, description="Modulate output by live audio loudness") reactive_audio_source_id: str | None = Field(None, description="AudioSource id for reactivity") - reactive_mode: str | None = Field(None, description="brightness | saturation | both") + reactive_mode: Literal["brightness", "saturation", "both"] | None = Field( + None, description="brightness | saturation | both" + ) reactive_intensity: Any = Field(default=None, description="Reactive modulation strength (0-1)") diff --git a/server/src/ledgrab/api/schemas/mqtt.py b/server/src/ledgrab/api/schemas/mqtt.py index ad2006d..8ed5531 100644 --- a/server/src/ledgrab/api/schemas/mqtt.py +++ b/server/src/ledgrab/api/schemas/mqtt.py @@ -20,7 +20,10 @@ class MQTTSourceCreate(BaseModel): default=False, description="Publish Home Assistant MQTT auto-discovery configs" ) discovery_prefix: str = Field( - default="homeassistant", description="HA MQTT discovery prefix (default 'homeassistant')" + default="homeassistant", + max_length=64, + pattern=r"^[A-Za-z0-9_\-/]+$", + description="HA MQTT discovery prefix (default 'homeassistant')", ) description: str | None = Field(None, description="Optional description", max_length=500) tags: List[str] = Field(default_factory=list, description="User-defined tags") @@ -49,7 +52,12 @@ class MQTTSourceUpdate(BaseModel): publish_ha_discovery: bool | None = Field( None, description="Publish Home Assistant MQTT auto-discovery configs" ) - discovery_prefix: str | None = Field(None, description="HA MQTT discovery prefix") + discovery_prefix: str | None = Field( + None, + max_length=64, + pattern=r"^[A-Za-z0-9_\-/]+$", + description="HA MQTT discovery prefix", + ) description: str | None = Field(None, description="Optional description", max_length=500) tags: List[str] | None = None icon: str | None = Field( diff --git a/server/src/ledgrab/core/devices/lifx_client.py b/server/src/ledgrab/core/devices/lifx_client.py index 07d8c98..5c7eee8 100644 --- a/server/src/ledgrab/core/devices/lifx_client.py +++ b/server/src/ledgrab/core/devices/lifx_client.py @@ -396,7 +396,7 @@ class LIFXClient(LEDClient): self._protocol.received.clear() self._send(MSG_GET_DEVICE_CHAIN, b"") self._send(MSG_GET_COLOR_ZONES, _build_get_color_zones_payload()) - loop = asyncio.get_event_loop() + loop = asyncio.get_running_loop() deadline = loop.time() + 0.6 while loop.time() < deadline: await asyncio.sleep(0.05) diff --git a/server/src/ledgrab/core/devices/nanoleaf_client.py b/server/src/ledgrab/core/devices/nanoleaf_client.py index 6742670..123701b 100644 --- a/server/src/ledgrab/core/devices/nanoleaf_client.py +++ b/server/src/ledgrab/core/devices/nanoleaf_client.py @@ -326,7 +326,7 @@ class NanoleafClient(LEDClient): """Stream per-panel (extControl) when enabled, else average to one HSB state.""" if not self.is_connected: raise RuntimeError("NanoleafClient not connected") - loop_now = asyncio.get_event_loop().time() + loop_now = asyncio.get_running_loop().time() if loop_now < self._next_tx_at: return True diff --git a/server/src/ledgrab/core/game_integration/runtime_state.py b/server/src/ledgrab/core/game_integration/runtime_state.py index 0761776..5e3ebc7 100644 --- a/server/src/ledgrab/core/game_integration/runtime_state.py +++ b/server/src/ledgrab/core/game_integration/runtime_state.py @@ -61,12 +61,22 @@ def record_events(integration_id: str, events: list[GameEvent]) -> None: def get_stats(integration_id: str) -> dict[str, Any]: - """Get runtime stats for an integration.""" + """Get a snapshot of runtime stats for an integration. + + Returns a fresh copy (incl. a copied ``event_counts_by_type``) so the caller + can read/iterate it on the event loop while the poll thread keeps mutating + the live dict under the lock — otherwise a concurrent insert raises + ``RuntimeError: dictionary changed size during iteration``. + """ with _state_lock: - return _integration_stats.get( - integration_id, - {"event_count": 0, "event_counts_by_type": {}, "last_event_time": None}, - ) + stats = _integration_stats.get(integration_id) + if stats is None: + return {"event_count": 0, "event_counts_by_type": {}, "last_event_time": None} + return { + "event_count": stats["event_count"], + "event_counts_by_type": dict(stats["event_counts_by_type"]), + "last_event_time": stats["last_event_time"], + } def cleanup_state(integration_id: str) -> None: diff --git a/server/src/ledgrab/core/mqtt/mqtt_manager.py b/server/src/ledgrab/core/mqtt/mqtt_manager.py index 8c48d04..78318a2 100644 --- a/server/src/ledgrab/core/mqtt/mqtt_manager.py +++ b/server/src/ledgrab/core/mqtt/mqtt_manager.py @@ -37,6 +37,12 @@ class MQTTManager: # Sources for which we hold a discovery acquire() reference. self._discovery_sources: set[str] = set() self._lock = asyncio.Lock() + # Serializes the discovery reconcile (ensure/disable) check-then-acquire so + # two concurrent sync_discovery() calls for the same source can't both see + # "first time" and double-acquire (ref-count leak). Separate from _lock + # because ensure/disable call acquire()/release() which take _lock, and + # asyncio.Lock is not re-entrant. + self._discovery_lock = asyncio.Lock() async def acquire(self, source_id: str) -> MQTTRuntime: """Get or create a runtime for the given MQTT source. Increments ref count.""" @@ -142,29 +148,31 @@ class MQTTManager: return if not getattr(source, "publish_ha_discovery", False): return - first_time = source_id not in self._discovery_sources - if first_time: - runtime = await self.acquire(source_id) - self._discovery_sources.add(source_id) - else: - runtime = self.get_runtime(source_id) - if runtime is None: - return + async with self._discovery_lock: + first_time = source_id not in self._discovery_sources + if first_time: + runtime = await self.acquire(source_id) + self._discovery_sources.add(source_id) + else: + runtime = self.get_runtime(source_id) + if runtime is None: + return await self._make_publisher(runtime, source).publish_all() async def disable_discovery(self, source_id: str) -> None: """Clear a source's discovery configs and drop our runtime reference.""" - if source_id not in self._discovery_sources: - return - runtime = self.get_runtime(source_id) - if runtime is not None: - try: - source = self._store.get(source_id) - await self._make_publisher(runtime, source).remove_all() - except Exception as exc: # noqa: BLE001 — best-effort cleanup - logger.warning("HA discovery cleanup failed for %s: %s", source_id, exc) - self._discovery_sources.discard(source_id) - await self.release(source_id) + async with self._discovery_lock: + if source_id not in self._discovery_sources: + return + self._discovery_sources.discard(source_id) + runtime = self.get_runtime(source_id) + if runtime is not None: + try: + source = self._store.get(source_id) + await self._make_publisher(runtime, source).remove_all() + except Exception as exc: # noqa: BLE001 — best-effort cleanup + logger.warning("HA discovery cleanup failed for %s: %s", source_id, exc) + await self.release(source_id) async def sync_discovery(self, source_id: str) -> None: """Reconcile discovery state after a source is created/updated.""" diff --git a/server/src/ledgrab/core/processing/effect_stream.py b/server/src/ledgrab/core/processing/effect_stream.py index feb1dd6..7d5023f 100644 --- a/server/src/ledgrab/core/processing/effect_stream.py +++ b/server/src/ledgrab/core/processing/effect_stream.py @@ -456,10 +456,10 @@ class EffectColorStripStream(ColorStripStream): Quiet audio dims/desaturates toward ``1 - intensity``; loud audio drives full brightness (and a saturation boost up to ``1 + intensity``). """ - e = self._audio_tap.energy() k = max(0.0, min(1.0, self.resolve("reactive_intensity", self._reactive_intensity))) if k <= 0.0: return + e = self._audio_tap.energy() f = buf.astype(np.float32) if self._reactive_mode in ("brightness", "both"): f *= (1.0 - k) + k * e diff --git a/server/src/ledgrab/static/css/modal.css b/server/src/ledgrab/static/css/modal.css index 47c510c..4e6ba48 100644 --- a/server/src/ledgrab/static/css/modal.css +++ b/server/src/ledgrab/static/css/modal.css @@ -3277,6 +3277,14 @@ background: none; cursor: pointer; flex-shrink: 0; + transition: box-shadow var(--duration-fast, 120ms) ease, + border-color var(--duration-fast, 120ms) ease; +} +.gradient-harmony-row input[type="color"]:hover, +.gradient-harmony-row input[type="color"]:focus-visible { + outline: none; + border-color: var(--primary-color, var(--border-color)); + box-shadow: 0 0 0 3px color-mix(in srgb, var(--primary-color, #4c8dff) 28%, transparent); } .gradient-harmony-types { display: flex; @@ -3284,7 +3292,7 @@ gap: 6px; } .gradient-harmony-btn { - padding: 4px 10px; + /* padding inherited from .btn-sm; only the slightly smaller harmony size differs */ font-size: 0.75rem; } @@ -3296,6 +3304,12 @@ gap: 8px; margin-top: 10px; } +/* The toggle's text label — neutralize the global `label` bottom margin so it + stays vertically centered against the switch in this flex row. */ +.calibration-linear-row label[for] { + margin: 0; + cursor: pointer; +} .calibration-linear-row .input-hint { flex-basis: 100%; margin: 0; diff --git a/server/src/ledgrab/static/js/global.d.ts b/server/src/ledgrab/static/js/global.d.ts index a34c8db..e9544c5 100644 --- a/server/src/ledgrab/static/js/global.d.ts +++ b/server/src/ledgrab/static/js/global.d.ts @@ -213,6 +213,7 @@ interface Window { cloneAutomation: (...args: any[]) => any; deleteAutomation: (...args: any[]) => any; copyWebhookUrl: (...args: any[]) => any; + triggerAutomationNow: (...args: any[]) => any; // ─── Scene Presets ─── openScenePresetCapture: (...args: any[]) => any; @@ -275,6 +276,7 @@ startTargetOverlay: (...args: any[]) => any; deleteColorStrip: (...args: any[]) => any; onCSSTypeChange: (...args: any[]) => any; onEffectTypeChange: (...args: any[]) => any; + onEffectReactiveToggle: (...args: any[]) => any; onCSSClockChange: (...args: any[]) => any; onAnimationTypeChange: (...args: any[]) => any; onDaylightRealTimeChange: (...args: any[]) => any; diff --git a/server/src/ledgrab/templates/modals/calibration.html b/server/src/ledgrab/templates/modals/calibration.html index 35b97c1..3754c61 100644 --- a/server/src/ledgrab/templates/modals/calibration.html +++ b/server/src/ledgrab/templates/modals/calibration.html @@ -199,7 +199,7 @@ - Linear-light blending + Average border pixels in linear light for perceptually correct, brighter colour mixing.
@@ -207,7 +207,7 @@ - Dithering + Spatio-temporal dithering reduces visible banding on smooth gradients.
diff --git a/server/src/ledgrab/utils/solar.py b/server/src/ledgrab/utils/solar.py index f57b46b..594d6f1 100644 --- a/server/src/ledgrab/utils/solar.py +++ b/server/src/ledgrab/utils/solar.py @@ -89,7 +89,11 @@ def utc_offset_hours_for(tz_name: str, when: datetime.datetime | None = None) -> offset = when.replace(tzinfo=None).astimezone(ZoneInfo(tz_name)).utcoffset() if offset is not None: return offset.total_seconds() / 3600.0 - except ZoneInfoNotFoundError: + # ZoneInfo() also raises ValueError (path-traversal / null-byte names) and + # OSError (over-long names) for malformed input, not just + # ZoneInfoNotFoundError. Catch all three so one bad SolarRule.timezone + # can't crash the whole automation evaluation tick. + except (ZoneInfoNotFoundError, ValueError, OSError): pass local_offset = when.astimezone().utcoffset() return local_offset.total_seconds() / 3600.0 if local_offset else 0.0 diff --git a/server/tests/core/test_automation_rule_handlers.py b/server/tests/core/test_automation_rule_handlers.py index 3657200..90f0dee 100644 --- a/server/tests/core/test_automation_rule_handlers.py +++ b/server/tests/core/test_automation_rule_handlers.py @@ -19,6 +19,7 @@ from ledgrab.storage.automation import ( DisplayStateRule, HomeAssistantRule, HTTPPollRule, + ManualTriggerRule, MQTTRule, Rule, SolarRule, @@ -30,6 +31,7 @@ from ledgrab.storage.automation import ( EXPECTED_RULE_TYPES = { StartupRule, + ManualTriggerRule, ApplicationRule, TimeOfDayRule, SolarRule, diff --git a/server/tests/test_release_review_2026_06_23.py b/server/tests/test_release_review_2026_06_23.py new file mode 100644 index 0000000..cf2c908 --- /dev/null +++ b/server/tests/test_release_review_2026_06_23.py @@ -0,0 +1,149 @@ +"""Regression tests for the 2026-06-23 pre-release review fixes. + +Covers the roadmap-batch (per-pixel smart-lights + integrations) findings fixed +before release: + +* solar timezone offset must not crash on a malformed ``timezone`` string + (DoS of the automation evaluation tick / 500 on manual trigger); +* the webhook action's ``content_type`` must reject CRLF / control chars + (outbound HTTP header injection); +* the MQTT ``discovery_prefix`` must reject wildcard / control chars + (HA-discovery topic injection); +* the effect ``reactive_mode`` must reject unknown values (silent no-op); +* game-integration ``get_stats`` must return an independent copy + (cross-thread ``dict changed size during iteration``). +""" + +import datetime +import types + +import pytest +from pydantic import ValidationError + +from ledgrab.api.routes.automations import _action_from_schema +from ledgrab.api.schemas.automations import ActionSchema +from ledgrab.api.schemas.color_strip_sources import EffectCSSCreate +from ledgrab.api.schemas.mqtt import MQTTSourceCreate +from ledgrab.core.game_integration import runtime_state +from ledgrab.utils.solar import utc_offset_hours_for + + +# ── solar timezone offset hardening ────────────────────────────────────────── + + +@pytest.mark.parametrize( + "bad_tz", + [ + "../../../etc/passwd", # path traversal -> ValueError + "foo\x00bar", # embedded null -> ValueError + "x" * 5000, # over-long -> OSError on some platforms + "Not/A/Real/Zone", # plausible-but-unknown -> ZoneInfoNotFoundError + ], +) +def test_solar_offset_does_not_raise_on_malformed_timezone(bad_tz): + """A crafted SolarRule.timezone must fall back, never crash the eval tick.""" + when = datetime.datetime(2026, 1, 15, 12, 0, 0) + offset = utc_offset_hours_for(bad_tz, when) + assert isinstance(offset, float) + + +def test_solar_offset_valid_timezone_still_resolves(): + when = datetime.datetime(2026, 1, 15, 12, 0, 0) # winter -> EST = -5 + assert utc_offset_hours_for("America/New_York", when) == pytest.approx(-5.0) + + +# ── webhook action Content-Type header injection ───────────────────────────── + + +@pytest.mark.parametrize( + "bad_ct", + [ + "application/json\r\nX-Injected: evil", + "text/plain\nX-Injected: evil", + "application/json\x00", + "application/jsön", # non-ASCII + ], +) +def test_webhook_action_rejects_unsafe_content_type(bad_ct): + schema = ActionSchema( + action_type="webhook", + webhook_url="http://10.0.0.5/hook", + method="POST", + content_type=bad_ct, + fire_on="activate", + ) + with pytest.raises(ValueError, match="content_type"): + _action_from_schema(schema) + + +def test_webhook_action_accepts_normal_content_type(): + schema = ActionSchema( + action_type="webhook", + webhook_url="http://10.0.0.5/hook", # LAN IP is allowed by SSRF policy + method="POST", + content_type="application/json; charset=utf-8", + fire_on="activate", + ) + action = _action_from_schema(schema) + assert action.content_type == "application/json; charset=utf-8" + + +# ── MQTT HA-discovery topic injection ──────────────────────────────────────── + + +@pytest.mark.parametrize( + "bad_prefix", + [ + "homeassistant/+/evil", # MQTT wildcard + "homeassistant/#", # MQTT wildcard + "home\nassistant", # control char + "x" * 65, # over max_length + ], +) +def test_mqtt_discovery_prefix_rejects_unsafe_value(bad_prefix): + with pytest.raises(ValidationError): + MQTTSourceCreate(name="src", broker_host="broker.local", discovery_prefix=bad_prefix) + + +def test_mqtt_discovery_prefix_defaults_to_homeassistant(): + src = MQTTSourceCreate(name="src", broker_host="broker.local") + assert src.discovery_prefix == "homeassistant" + + +# ── effect reactive_mode validation ────────────────────────────────────────── + + +def test_reactive_mode_rejects_unknown_value(): + with pytest.raises(ValidationError): + EffectCSSCreate(name="fx", reactive_mode="invalid") + + +@pytest.mark.parametrize("mode", ["brightness", "saturation", "both"]) +def test_reactive_mode_accepts_known_values(mode): + src = EffectCSSCreate(name="fx", reactive_mode=mode) + assert src.reactive_mode == mode + + +# ── game-integration get_stats returns an independent snapshot ─────────────── + + +def test_get_stats_returns_independent_copy(): + integration_id = "test-int-copy" + runtime_state.cleanup_state(integration_id) + try: + event = types.SimpleNamespace(event_type="kill", timestamp="2026-06-23T00:00:00Z") + runtime_state.record_events(integration_id, [event]) + + snapshot = runtime_state.get_stats(integration_id) + assert snapshot["event_count"] == 1 + assert snapshot["event_counts_by_type"] == {"kill": 1} + + # Mutating the returned snapshot must not corrupt the live state. + snapshot["event_counts_by_type"]["kill"] = 999 + snapshot["event_counts_by_type"]["injected"] = 1 + + fresh = runtime_state.get_stats(integration_id) + assert fresh["event_counts_by_type"] == {"kill": 1} + assert "injected" not in fresh["event_counts_by_type"] + finally: + runtime_state.cleanup_state(integration_id)