Rotation loop permanently dies on a malformed rotation_interval_seconds value #11

New issue

Closed

opened 2026-07-28 21:40:28 +00:00 by paulloth1 · 1 comment

paulloth1 commented

2026-07-28 21:40:28 +00:00

(Migrated from github.com)

Problem

/admin/settings accepts rotation_interval_seconds as a raw string with no server-side validation (type="number" on the input is just an HTML hint, not enforced). Whatever is submitted gets stored as-is via db.set_setting().

backend/rotation.py's rotation_loop() and _render_forced() both do int(await db.get_setting("rotation_interval_seconds", "60")) with no try/except. If the stored value isn't a valid int (empty string, non-numeric, etc.), this raises ValueError inside the one-and-only rotation task. Nothing catches it, and backend/app.py's lifespan() only calls asyncio.create_task(rotation_loop()) once — there's no supervisor/restart logic. The task just dies silently (asyncio logs "exception was never retrieved" but the process keeps running otherwise), and the display freezes on whatever was showing until the service is manually restarted.

Since the admin UI is intentionally open to anyone on the network (by design, see #3), this is an easy way — accidental or not — to take the whole signage down with a single bad form submission.

Suggested fix

Validate rotation_interval_seconds server-side in the /admin/settings handler (reject/clamp non-numeric or out-of-range values with a friendly error instead of storing them).
Defensively wrap the int(...) calls in rotation.py with a fallback default in case a bad value ever does slip through.

## Problem `/admin/settings` accepts `rotation_interval_seconds` as a raw string with no server-side validation (`type="number"` on the input is just an HTML hint, not enforced). Whatever is submitted gets stored as-is via `db.set_setting()`. `backend/rotation.py`'s `rotation_loop()` and `_render_forced()` both do `int(await db.get_setting("rotation_interval_seconds", "60"))` with no try/except. If the stored value isn't a valid int (empty string, non-numeric, etc.), this raises `ValueError` **inside the one-and-only rotation task**. Nothing catches it, and `backend/app.py`'s `lifespan()` only calls `asyncio.create_task(rotation_loop())` once — there's no supervisor/restart logic. The task just dies silently (asyncio logs "exception was never retrieved" but the process keeps running otherwise), and the display freezes on whatever was showing until the service is manually restarted. Since the admin UI is intentionally open to anyone on the network (by design, see #3), this is an easy way — accidental or not — to take the whole signage down with a single bad form submission. ## Suggested fix - Validate `rotation_interval_seconds` server-side in the `/admin/settings` handler (reject/clamp non-numeric or out-of-range values with a friendly error instead of storing them). - Defensively wrap the `int(...)` calls in `rotation.py` with a fallback default in case a bad value ever does slip through.

paulloth1 commented

2026-07-28 21:43:32 +00:00

(Migrated from github.com)

Fixed in 73f0db2: added _get_interval() in rotation.py which falls back to the 60s default on any non-numeric/non-positive stored value instead of raising; also wrapped rotation_loop()'s body in a top-level try/except so any future unexpected error retries after 5s instead of permanently killing the task. Added server-side validation on POST /admin/settings — non-numeric or non-positive submissions are now silently rejected (old value kept) rather than stored. Verified live: submitting rotation_interval_seconds=garbage via the LAN port left the old value (20) in place and rotation kept ticking normally.

Fixed in 73f0db2: added _get_interval() in rotation.py which falls back to the 60s default on any non-numeric/non-positive stored value instead of raising; also wrapped rotation_loop()'s body in a top-level try/except so any future unexpected error retries after 5s instead of permanently killing the task. Added server-side validation on POST /admin/settings — non-numeric or non-positive submissions are now silently rejected (old value kept) rather than stored. Verified live: submitting rotation_interval_seconds=garbage via the LAN port left the old value (20) in place and rotation kept ticking normally.