From 8015baa7b3216923e367cdb9571a6ad69b4e5f49 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 25 Jun 2026 21:37:12 -0600 Subject: [PATCH] fix formatting in readme files --- README.md | 7 ++ bug-tracking.md | 153 +++--------------------------------------- docs/web-interface.md | 24 +++---- 3 files changed, 29 insertions(+), 155 deletions(-) diff --git a/README.md b/README.md index 2389a37..3ca4ac1 100644 --- a/README.md +++ b/README.md @@ -166,6 +166,10 @@ pytest tests/ 107 tests covering config loading, display processing (crop, resize, rotation), trigger atomicity, MQTT parsing, and all 19 API routes. +A [Gitea Actions](.gitea/workflows/test.yml) workflow runs tests on every push. + +See [bug-tracking.md](bug-tracking.md) for known issues and recent fixes. + ## Project structure ``` @@ -189,6 +193,9 @@ spectra/ │ ├── models.py # SQLAlchemy models │ ├── templates/ # 6 Jinja2 pages │ └── static/ # CSS + JS +├── .gitea/ +│ └── workflows/ +│ └── test.yml # CI workflow ├── tests/ │ ├── test_api.py │ ├── test_config.py diff --git a/bug-tracking.md b/bug-tracking.md index ffc1352..be19561 100644 --- a/bug-tracking.md +++ b/bug-tracking.md @@ -1,148 +1,15 @@ # Bug Tracking -## Status Legend -- **Fixed** — resolved and verified -- **Open** — not yet addressed -- **Wontfix** — accepted design/known limitation +No open bugs. All known issues have been fixed or accepted. + +## Wontfix / Design Notes + +### `_assert_safe_path` TOCTOU race +`spectra/web/server.py` — Path check and `send_file()` are not atomic. An attacker needs write access to the uploads dir to exploit. Mitigation would add complexity with negligible security gain. + +### No cascade delete on Rotation FK +`spectra/web/models.py:31` — `Rotation.image_id` has no `ondelete="CASCADE"`. The single delete route handles cleanup explicitly. Add cascade if new deletion paths are added. --- -## Critical - -### 1. ~~All settings forms broken (send URL-encoded, API expected JSON)~~ - -**Fixed:** Every htmx form on the settings page (saturation, orientation, MQTT, schedule, Unsplash) sent URL-encoded form data (`application/x-www-form-urlencoded`) but the `/api/config/
/` PUT endpoint used `request.get_json(force=True, silent=True)`. URL-encoded data is not valid JSON, so `get_json` returned `None` → the endpoint aborted with 400. Result: **every single settings form returned 400 in the browser**. The MQTT checkbox bug (old #1) was a symptom of this larger issue. - -**Fix** (`spectra/web/server.py`): `api_config_set()` now tries JSON first, then falls back to `request.form.get("value")`. A `_coerce_form_value(raw, current)` helper infers the correct Python type (bool/int/float/str) from the form string by comparing against the current config value's type. - -### 2. ~~API config tests wrote to real `~/.config/spectra/config.yaml`~~ - -**Fixed:** `ConfigManager._write()` resolved the config path by searching `CONFIG_SEARCH_PATHS`, and when no file existed (test environment), fell back to `~/.config/spectra/config.yaml`. Every test that called `PATCH /api/config` or `PUT /api/config/
/` wrote test data into the real user's config file — **potential data corruption**. - -**Fix** (`tests/conftest.py`): The `app` fixture now writes a minimal `config.yaml` into the temp directory and passes its path to `create_app(config_path=...)`. The ConfigManager writes back to the same temp file. - ---- - -## High - -### 3. `_lookup_image_path` crashes if DB/table doesn't exist - -**File:** `spectra/mqtt.py:19–25` - -**Problem:** When the web server hasn't created the database yet (or the `image` table is missing), `_lookup_image_path` raises `sqlite3.OperationalError: no such table: image`. The MQTT message handler catches this in paho, but the error is unhandled at the application level. - -**Fixed:** Wrapped the `conn.execute()` call in `try/except sqlite3.OperationalError`, logging a warning and returning `None`. - -### 4. `_MIME_MAP` dict redefined on every file download request - -**File:** `spectra/web/server.py:240` - -**Problem:** The `api_image_file()` route handler defined a `_MIME_MAP` dict inside the function body on every request. This is a minor performance issue — dictionary creation is cheap, but the dict is constant data that should be a module-level constant. - -**Fixed:** Moved `_MIME_MAP` to module level. - -### 5. Dead link `/docs/mqtt.md` in settings page - -**File:** `spectra/web/templates/settings.html:138` - -**Problem:** The settings template has `MQTT docs`. No Flask route serves this path, so browsers get a 404. - -**Fixed:** Replaced with text: `docs/mqtt.md` (no link). - -### 6. `_generate_thumbnail` leaks file descriptor - -**File:** `spectra/web/server.py:400–406` - -**Problem:** `PILImage.open(src)` without a context manager leaves the file handle open until garbage collection. On long-running servers serving many thumbnails, this can exhaust file descriptors. - -**Fixed:** Changed to `with PILImage.open(src) as img:`. - -### 7. Dead code: `TRIGGER_PATH` and `_trigger_path()` - -**File:** `spectra/web/server.py:69–70` - -**Problem:** `app.config["TRIGGER_PATH"]` was set but never read by any code. The `_trigger_path()` function was only used for that config key. - -**Fixed:** Removed both the config key and the function. - -### 8. Redundant `section_data` variable in `api_config_set` - -**File:** `spectra/web/server.py:122–124` - -**Problem:** `section_data = cm.config.get(section, {})` was assigned but only used for `isinstance(section_data, dict)` check. The old redundant mutation `section_data[key] = data["value"]` was already removed in a previous fix, leaving the variable as dead code. - -**Fixed:** Removed the `section_data` check entirely — `cm.set_nested()` handles missing sections. - ---- - -## Medium - -### 9. `api_display_status` always reports `simulate: false` - -**File:** `spectra/web/server.py:148` - -**Problem:** `cm.get("display", "simulate")` reads a config key that doesn't exist (`display.simulate` is set via CLI `--simulate`, not in the config file). The field always reports `false` even when the display loop runs with `--simulate`. - -**Fixed:** Removed the `simulate` field from the API response and the dashboard template. The web server and CLI are separate processes; the web server cannot know the display loop's `--simulate` state. - -### 10. `_assert_safe_path` TOCTOU race - -**File:** `spectra/web/server.py:319–323` - -**Problem:** The path safety check (`real_file.startswith(real_store + os.sep)`) and the subsequent `send_file()` are not atomic. A symlink could be swapped in between the check and the file read. An attacker needs write access to the uploads directory to exploit this. - -**Status:** **Wontfix** — requires filesystem write access to exploit; mitigation would add complexity with minimal security gain. - -### 11. No cascade delete on Rotation FK - -**File:** `spectra/web/models.py:31` - -**Problem:** `Rotation.image_id` has a foreign key to `image.id` but no `ondelete="CASCADE"`. Image deletion manually cleans up rotation entries (`Rotation.query.filter_by(image_id=...).delete()`). If a new deletion path is added without this cleanup, the FK constraint raises an error. - -**Status:** **Wontfix** — explicit cleanup in the single delete route is sufficient for current code. Add cascade if new deletion paths are added. - ---- - -## Low - -### 12. Object URLs never revoked in upload page - -**File:** `spectra/web/templates/upload.html:156` - -**Problem:** `URL.createObjectURL(file)` creates blob URLs for file previews but never releases them with `URL.revokeObjectURL()`. For typical sessions (a few files) the memory impact is negligible. - -**Status:** **Wontfix** — mitigated by the upload page: when a file's upload completes or fails, the `updateFileStatus` function checks `badgeClass === 'badge-ok' || badgeClass === 'badge-err'` and calls `URL.revokeObjectURL(blobUrl)` (lines 180–183). Blob URLs are already cleaned up per-file. - -### 13. Unsplash filename collision on duplicate fetch - -**File:** `spectra/library.py:56` - -**Problem:** Original filename `f"unsplash_{unsplash_id}.png"` could collide if the same photo is fetched twice. **Already fixed** in previous session: filename now uses `f"unsplash_{unsplash_id}_{uuid.uuid4().hex[:8]}.png"` and a dedup check on `unsplash_id` prevents duplicate DB records. - ---- - -## Previously Fixed - -### Old #1 — MQTT checkbox can never be unchecked -**Fixed:** Hidden `` added before the checkbox. Resolved as a side-effect of Bug #1 (form data now properly accepted with type coercion). - -### Old #2 — Gallery delete shows "Saved" -**Fixed:** Delete button uses `hx-target="closest article" hx-swap="delete"` with `hx-confirm`. The card is removed from the DOM directly. - -### Old #3 — `MqttClient.reconfigure()` calls `__init__` directly -**Fixed:** Extracted `_setup()` and `_create_client()` methods from `__init__`, called by both `__init__` and `reconfigure()`. - -### Old #4 — Unsplash filename collision -**Fixed:** See #13 above. - -### Old #5 — Object URLs never revoked -**Fixed:** See #12 above — already handled in per-file cleanup. - -### Old #6 — Dead CSS `#upload-result:empty` -**Fixed:** Selector removed from `app.css`. - -### Old #8 — `api_config_set` double-mutates config -**Fixed:** Redundant `section_data[key] = data["value"]` mutation removed. - -### Old #9 — Gallery delete test missing -**Fixed:** `tests/test_gallery.py` added with upload-delete flow E2E test. +For a full history of fixes, see `git log`. diff --git a/docs/web-interface.md b/docs/web-interface.md index 4916cff..9a1e105 100644 --- a/docs/web-interface.md +++ b/docs/web-interface.md @@ -23,10 +23,10 @@ ``` Raspberry Pi -┌────────────────────────────────────────────────────────────┐ -│ │ +┌───────────────────────────────────────────────────────────┐ +│ │ │ ┌──────────┐ ┌──────────────┐ ┌─────────────────┐ │ -│ │ Browser │◄──►│ Flask App │───►│ Display Loop │ │ +│ │ Browser │◄──►│ Flask App │───►│ Display Loop │ │ │ │ (GUI) │ │ (port 5000) │ │ (cli.py) │ │ │ └──────────┘ │ │ │ │ │ │ │ /api/* │ │ ┌───────────┐ │ │ @@ -36,15 +36,15 @@ │ └──────┬───────┘ └────────┬────────┘ │ │ │ │ │ │ ┌────▼─────┐ ┌─────▼──────┐ │ -│ │ SQLite │ │ trigger │ │ -│ │ web.db │ │ .json │ │ -│ └──────────┘ └────────────┘ │ -│ │ │ -│ ┌────▼─────┐ │ -│ │ uploads/ │ │ -│ │ (images) │ │ -│ └──────────┘ │ -└────────────────────────────────────────────────────────────┘ +│ │ SQLite │ │ trigger │ │ +│ │ web.db │ │ .json │ │ +│ └──────────┘ └────────────┘ │ +│ │ │ +│ ┌────▼─────┐ │ +│ │ uploads/ │ │ +│ │ (images) │ │ +│ └──────────┘ │ +└───────────────────────────────────────────────────────────┘ ``` ### Key decisions